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

UI: Fix recommendation test to ensure CPU exists #10004

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

backspace
Copy link
Contributor

This fixes a flaky test, as seen in this failure mentioned here.

One part of the test involves toggling off all memory recommendations
and then accepting, but it’s not possible to accept when there are
no CPU recommendations to begin with, which can happen because
there’s a 10% chance of not creating a corresponding recommendation
in the task factory. Since two tasks are created for this module, it’s
only a 1% chance of no CPU task, but that means 1% flakiness!

I confirmed this was the problem by using faker-seed=69400 and
await this.pauseTest():

recommendation-test-failure

With this mitigation in place, the same seed has a CPU recommendation and the
summary can therefore still be accepted with memory toggled off:

recommendation-success

Another approach would be to have a flag to remove randomness when
creating recommendations, but this one-time intervention seems preferable
to having to pass that down through the factories, to me 🤔

This fixes a flaky test, as seen in this failure:
https://app.circleci.com/pipelines/github/hashicorp/nomad/14726/workflows/f4ae0bf2-0699-4d18-b55e-5221aafe393c/jobs/137128

One part of the test involves toggling off all memory recommendations
and then accepting, but it’s not possible to accept when there are
no CPU recommendations to begin with.
@github-actions
Copy link

Ember Asset Size action

As of 5c71131

Files that stayed the same size 🤷‍:

File raw gzip
nomad-ui.js 0 B 0 B
vendor.js 0 B 0 B
nomad-ui.css 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

Ember Test Audit comparison

master 5c71131 change
passes 1555 1555 0
failures 0 0 0
flaky 0 0 0
duration 8m 00s 595ms 7m 27s 839ms -32s 756ms

Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz left a comment

Choose a reason for hiding this comment

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

I like this intervention! The test remains true to its intent and factories don't get any extra spaghetti.

@backspace backspace merged commit cb4443d into master Feb 16, 2021
@backspace backspace deleted the b-ui/recommendation-cpu-flaky branch February 16, 2021 14:43
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants