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

Replace hardcoded deploymentId values in airnode-node tests #1653

Merged
merged 3 commits into from
Feb 20, 2023

Conversation

dcroote
Copy link
Contributor

@dcroote dcroote commented Feb 17, 2023

Closes #1641.

deriveDeploymentId takes as one argument the Airnode version. Consequently, when the Airnode version is bumped, the deploymentId changes, which breaks tests that have hardcoded values. This PR replaces these hardcoded values with a regex so bumping the version no longer causes the tests to fail. Tests on the output of deriveDeploymentId are already present elsewhere.

@dcroote dcroote requested a review from amarthadan February 17, 2023 05:30
@dcroote dcroote self-assigned this Feb 17, 2023
expect(callApisSpy).toHaveBeenCalledWith(
[],
expect.objectContaining({
deploymentId: expect.stringMatching(new RegExp(`local[0-9a-z]{${DEPLOYMENT_ID_LENGTH}}`)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the regular expression is always the same we can extract it as a constant and use that instead.
Also, it can be made more strict using [0-9a-f] (only hexadecimal) instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A constant across all tests? Without the local prefix? I may not be understanding how you envision this.

Also a-f, right haha.

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 updated it to a-f and to have a const regex variable per test file. I don't think it makes sense to have it as a "global" constant because the cloud provider type prefix of local applies to these tests and wouldn't apply to aws or gcp deployment ID regexes

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean it as a global test constant since all the tests here use only the local prefix. But this is fine as well.

packages/airnode-node/src/constants.ts Outdated Show resolved Hide resolved
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.

Fix airnode-node tests breaking on package version update
2 participants