Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Blocktack Browser test refactored in Serenity #1955

Open
wants to merge 69 commits into
base: master
Choose a base branch
from

Conversation

timstackblock
Copy link
Contributor

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.

@CLAassistant
Copy link

CLAassistant commented Sep 8, 2019

CLA assistant check
All committers have signed the CLA.

@yknl yknl added this to the DX Q3 Sprint 3 milestone Sep 24, 2019
@stackatron stackatron added this to the DevX 2019 Q4 - Sprint 3 milestone Dec 3, 2019
@timstackblock
Copy link
Contributor Author

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.

@timstackblock
Copy link
Contributor Author

@yknl @jeffdomke @zone117x please review and merge when ready

.circleci/config.yml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
Copy link
Collaborator

@yknl yknl left a 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?

.circleci/oldConfig.yml Outdated Show resolved Hide resolved
this.Then(/^validate blockstack putFile$/, async () => {
if (browserName === 'safari') {
//do nothing due bug on IOS
//https://github.com/appium/appium/issues/13013
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

@zone117x zone117x Jan 11, 2020

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.

Copy link
Contributor Author

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

Copy link
Contributor

@zone117x zone117x Jan 15, 2020

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.

.circleci/config.yml Show resolved Hide resolved
@timstackblock
Copy link
Contributor Author

Ready to merge

@hstove
Copy link
Collaborator

hstove commented Jan 16, 2020

@timstackblock let's keep the discussion here, instead of using a new issue to discuss removing unit tests.

The unit test are not working this is a place holder top get the e2e test working for the browser.

I'm not sure I fully understand. We currently (in master) have a build job that runs separately from all the E2E tests. This job is responsible for our unit tests - which we definitely need to keep. Please give an example of how the unit tests could cause the e2e tests to fail, if possible. Also, in master and in other branches, the unit tests are working and passing.

Copy link
Contributor

@zone117x zone117x left a 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants