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

[Node.js upgrade] Skips flaky server metrics collector tests #1027

Closed

Conversation

tmarkley
Copy link
Contributor

Description

  • Adds --no-deprecation flag for integration tests caused by shot
    which is a downstream dependency of hapi.
  • The ServerMetricsCollector tests are flaky and rely on the existing
    v17 hapi library that Dashboards depends on. This will be upgraded
    for the 2.0 release along with the Node.js upgrade.

Issues Resolved

Resolves #991

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

* Adds `--no-deprecation` flag for integration tests caused by `shot`
which is a downstream dependency of `hapi`.
* The ServerMetricsCollector tests are flaky and rely on the existing
v17 hapi library that Dashboards depends on. This will be upgraded
for the 2.0 release along with the Node.js upgrade.

Signed-off-by: Tommy Markley <markleyt@amazon.com>
@tmarkley tmarkley added test:integration build Build related additions or modifications >upgrade labels Dec 13, 2021
@tmarkley tmarkley requested a review from a team December 13, 2021 22:48
@tmarkley tmarkley changed the title Skips flaky server metrics collector tests [Node.js upgrade] Skips flaky server metrics collector tests Dec 13, 2021
@tmarkley tmarkley linked an issue Dec 13, 2021 that may be closed by this pull request
Copy link
Contributor

@boktorbb boktorbb left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -42,7 +42,7 @@
"opensearch": "node scripts/opensearch",
"test": "grunt test",
"test:jest": "node scripts/jest",
"test:jest_integration": "node scripts/jest_integration",
"test:jest_integration": "node --no-deprecation scripts/jest_integration",
Copy link
Member

Choose a reason for hiding this comment

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

What's the delta in tests after adding this? Like before change count and after change count.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There shouldn't be a difference since the flag just ignores deprecation warnings. The tests don't run without it at this point.

@tmarkley
Copy link
Contributor Author

Covered in #1028

@tmarkley tmarkley closed this Dec 14, 2021
@tmarkley tmarkley deleted the integration-tests branch February 28, 2022 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build related additions or modifications test:integration >upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Node.js upgrade] Fix failing integration tests
3 participants