Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'ballot' and 'compiling' browser tests under nightwatch_local #2025

Conversation

scottt
Copy link
Contributor

@scottt scottt commented May 25, 2019

The testSignature() browser test in generalTests.js would
occasionally fail due to not obtaining the correct hash and
signature values from the browser, resulting in:

✖ generalTests
   - Simple Contract (40.954s)
   Failed [equal]: ('' == '0: address: 0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c')  - expected "0: address: 0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c" but got: ""
       at Object.<anonymous> (/home/scottt/work/remix-0.8/remix-ide/test-browser/helpers/contracts.js:129:22)

failures.

This patch fixes it by waiting for the DOM elements with
waitForElementPresent which implicitly retries and obtaining
the correct contract instance address instead of hard coding it.

@scottt scottt force-pushed the scottt-fix-nightwatch-local-ballot-and-compiling-tests branch 3 times, most recently from 673dfa7 to c8b84c2 Compare May 27, 2019 14:24
@yann300
Copy link
Collaborator

yann300 commented May 29, 2019

Thanks for the PR,
We have fixed the issue and test should pass now, we'd prefer not to modify the browser test
But that said, it makes sense to include your PR because it contains some nice improvement.
Could you remove the calls to perform(contractHelper.waitForValue('#txorigin', 10)) and keep the rest, we then merge it.
cc @LianaHus

will have a look on the evmVersion shortly

The `testSignature()` browser test in `generalTests.js` would
occasionally fail due to not obtaining the correct `hash` and
`signature` values from the browser, resulting in:
```
✖ generalTests
   - Simple Contract (40.954s)
   Failed [equal]: ('' == '0: address: 0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c')  - expected "0: address: 0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c" but got: ""
       at Object.<anonymous> (/home/scottt/work/remix-0.8/remix-ide/test-browser/helpers/contracts.js:129:22)
```
failures.

This patch fixes it by waiting for the DOM elements with
`waitForElementPresent` which implicitly retries and obtaining
the correct contract instance address instead of hard coding it.
@scottt scottt force-pushed the scottt-fix-nightwatch-local-ballot-and-compiling-tests branch from c8b84c2 to d0ddd20 Compare May 29, 2019 12:37
@scottt
Copy link
Contributor Author

scottt commented May 29, 2019

Could you remove the calls to perform(contractHelper.waitForValue('#txorigin', 10)) and keep the rest, we then merge it.

Done! :)

We have fixed the issue and test should pass now, we'd prefer not to modify the browser test

I believe you're referring to:
0351eb1
which makes waitForValue('#txorigin') no longer necessary by changing the way the account list is filled?

will have a look on the evmVersion shortly

I really appreciate the patch reviews and the work you guys put into this open source project.

@LianaHus LianaHus merged commit 214c4fb into ethereum:master May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants