Skip to content

test: don't hardcode timeout interval and don't rely on jasmine #4646

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

Merged
merged 2 commits into from
Jul 20, 2022

Conversation

paul-soporan
Copy link
Member

@paul-soporan paul-soporan commented Jul 20, 2022

What's the problem this PR addresses?

#4543 hardcoded the timeout interval, but it has already gotten out of sync due to #3669.

How did you fix it?

Made it use jasmine.DEFAULT_TIMEOUT_INTERVAL Extracted the timeout to pkg-tests-core and removed usage of jasmine.DEFAULT_TIMEOUT_INTERVAL to make it easier for us to switch to jest-circus, the new jest runner.

I also added a TODO so that we don't forget to recheck whether this is still needed. (I have a feeling that the big-endian PR might've fixed all timeout issues).

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@arcanis
Copy link
Member

arcanis commented Jul 20, 2022

Is jasmine.DEFAULT_TIMEOUT_INTERVAL available under jest-circus, the new Jest test runner that's been made the default?

@paul-soporan
Copy link
Member Author

It's not, I updated the tests to not rely on jasmine (my BE PR used jasmine.DEFAULT_TIMEOUT_INTERVAL).

Will look into switching our tests to jest-circus in a different PR.

@paul-soporan paul-soporan changed the title test: don't hardcode timeout interval test: don't hardcode timeout interval and don't rely on jasmine Jul 20, 2022
@paul-soporan paul-soporan merged commit eed9941 into master Jul 20, 2022
@paul-soporan paul-soporan deleted the paul/test/dont-hardcode-timeout-interval branch July 20, 2022 18:21
@merceyz
Copy link
Member

merceyz commented Jul 20, 2022

Will look into switching our tests to jest-circus in a different PR.

Updating Jest would do that automatically.

@paul-soporan
Copy link
Member Author

Yep, I'll work on upgrading jest and gradually enabling new features to see how much it improves things.

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