-
Notifications
You must be signed in to change notification settings - Fork 199
Blocktack Browser test refactored in Serenity #1955
base: master
Are you sure you want to change the base?
Conversation
I have isolated the issue that is causing iOS to fail occasionally on browserstack. See the github link to the known issue that causes test flakes appium/appium#13013 . It seems there is a known bug with appium and ios devices when executing async script query. In some scenarios in our test we are using async script query to get some info about objects in DOM. This is unusual for browser test because they usually just mimic user action but our test also execute Blockstack JS unit test. In IOS this causes the occasional long delays and unexpected errors. Examples: browser wasn't created, or long execution of javascript requests in tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the files in test-e2e/target/site/
automatically generated? If so can we leave them out of git?
this.Then(/^validate blockstack putFile$/, async () => { | ||
if (browserName === 'safari') { | ||
//do nothing due bug on IOS | ||
//https://github.com/appium/appium/issues/13013 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to have fixed now -- have you tried with their latest packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah its still now working correctly they claim its fixed but its still flakes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it just fails occasionally, I think it should be be enabled. If it has to be disabled for iOS, it should still be enabled for MacOS Safari.
This is currently the only automated E2E test suite for blockstack.js. While not an ideal setup, we should try to preserve it, until the blockstack.js repo has its own E2E test suite for supported web browsers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it fails about 80% of the time you would never be able to get a successful run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it fail for desktop Safari? It looks like an iOS only issue. This removes all integration tests for Blockstack Browser and blockstack.js for the Safari web browser, which I strongly think we should keep.
...e/target/site/serenity/576946480b52ad056d6f5bddf874399c83582ecf90963cc074a14c70580e7d9f.html
Outdated
Show resolved
Hide resolved
Ready to merge |
@timstackblock let's keep the discussion here, instead of using a new issue to discuss removing unit tests.
I'm not sure I fully understand. We currently (in master) have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several important integration tests that are disabled for Safari. It looks like the issue is for iOS, so they should still be ran for desktop Safari.
It's important to maintain these because its currently the only way blockstack.js gets any cross-browser integration testing.
The Blockstack browser E2E test have been refactored to run in serenity. The change should help reduce flakey test errors and make the test suite more stable. I am still finalizing CI test integration.