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

Skip running TokenField tests by default #982

Merged
merged 1 commit into from
Jun 1, 2017

Conversation

nylen
Copy link
Member

@nylen nylen commented Jun 1, 2017

Follow-up to #924 (comment)

Even though these tests use enzyme, they look more like "integration tests" than "unit tests" and perhaps should not be run on every build. They might fit better in another category of tests (#939 (comment)), with a different strategy for running them (for example, executed every hour against the master branch).

However, TokenField has a long history of weird bugs and these tests are pretty important to keep around in some form.

Also worth noting that these tests execute far, far more quickly on my computer than they do on Travis. The slowest test when running locally was 639ms; on Travis this is more like 3 seconds.

@mtias mtias added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Jun 1, 2017
@aduth
Copy link
Member

aduth commented Jun 1, 2017

Also worth noting that these tests execute far, far more quickly on my computer than they do on Travis. The slowest test when running locally was 639ms; on Travis this is more like 3 seconds.

For me, seven of the tests are marked by Mocha as "slow" when run locally, ranging from about 1-2.5s.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree we should stop these tests from running 'til we can resolve timeouts 👍

@nylen
Copy link
Member Author

nylen commented Jun 1, 2017

I am fully in favor of having all of our standard test suite execute in less than one second (after build time).

However, it's a bit worrisome that these tests are enough to push our builds over the timeout limit. This must mean that we're already pretty close. I wonder how much upgrading to npm 5 would help with this.

@nylen nylen merged commit 97ee1a3 into master Jun 1, 2017
@nylen nylen deleted the tests/skip-tokenfield-by-default branch June 1, 2017 18:59
@BE-Webdesign
Copy link
Contributor

Are the tests using real timeOuts or what is the source of them taking so long? Also big 👍.

@nylen
Copy link
Member Author

nylen commented Jun 1, 2017

I think it's just that jsdom is slow at emulating mouse and keyboard events (and that Travis is much slower than Circle CI, where these tests run today in Calypso). We're not using any timeouts in the tests.

it's a bit worrisome that these tests are enough to push our builds over the timeout limit

Or were we running into the mocha default timeout of 2 seconds? I'm not sure I understand what the problem was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants