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: increase cli test timeout #952

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Conversation

gitphill
Copy link
Contributor

@gitphill gitphill commented Jan 8, 2020

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Increase tap timeout from 300 to 400 due to increased test count we're
regularly hitting this timeout.

How should this be manually tested?

npm test:acceptance

@gitphill gitphill requested a review from a team January 8, 2020 16:14
@gitphill gitphill self-assigned this Jan 8, 2020
@gitphill gitphill force-pushed the fix/increase-cli-test-timeout branch 3 times, most recently from fd20149 to f739336 Compare January 8, 2020 18:32
package.json Outdated
@@ -29,7 +29,7 @@
"prepare": "npm run build",
"test:common": "npm run check-tests && npm run build && npm run lint && node --require ts-node/register src/cli test --org=snyk",
"test:acceptance": "tap test/acceptance/**/*.test.* -Rspec --timeout=300 --node-arg=-r --node-arg=ts-node/register",
"test:system": "tap test/system/*.test.* -Rspec --timeout=300 --node-arg=-r --node-arg=ts-node/register",
"test:system": "tap test/system/*.test.* -Rspec --node-arg=-r --node-arg=ts-node/register",
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty sure leaving this out means a default timeout of 60 seconds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've tried all sorts (up to 600ms), starting to think timeout is not the issue

@gitphill gitphill force-pushed the fix/increase-cli-test-timeout branch from d888822 to 85e11a2 Compare January 8, 2020 19:39
@snyk snyk deleted a comment from snyksec Jan 8, 2020
@ghost ghost requested review from dkontorovskyy and orsagie January 8, 2020 20:57
@gitphill gitphill force-pushed the fix/increase-cli-test-timeout branch from ecd5d5e to 8b669a3 Compare January 8, 2020 22:49
@gitphill gitphill added the 🚧 WIP Work In Progress label Jan 9, 2020
Open authenticate window immediately.
@gitphill gitphill force-pushed the fix/increase-cli-test-timeout branch from e7230ec to c6f0b28 Compare January 9, 2020 12:03
@gitphill gitphill removed the 🚧 WIP Work In Progress label Jan 9, 2020
@gitphill
Copy link
Contributor Author

gitphill commented Jan 9, 2020

OK after lots of experimentation, the real issue was the amount of time we take to open the browser window. Reduced this from 2 seconds to immediate. Allows tests to more consistently pass and not timeout. The timeout was cause because the test teardown brought down the local server before all async tests had returned.

@gitphill gitphill merged commit 283b29b into master Jan 9, 2020
@gitphill gitphill deleted the fix/increase-cli-test-timeout branch January 9, 2020 14:28
@snyksec
Copy link

snyksec commented Jan 9, 2020

🎉 This PR is included in version 1.277.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants