fixing flaky vsualize data_table_nontimeindex test #22288
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ppisljar
added
review
test
Feature:Visualizations
💔 Build Failed |
|
retest |
kobelb
approved these changes
Aug 23, 2018
💔 Build Failed |
markov00
requested changes
Aug 23, 2018
There was a problem hiding this comment.
This will fix the flaky test on data_table, but introduced the error on the gauge (not present on master).
Not sure what changed between the two but seems that test should show correct values for fields with fieldFormatters fails because clickMetricEditor toggle off the Metric editor and the subsequent clickBucket doesn't find Split Group liste element because it's already configured and the list of available buckets is already gone.
I think we need to change the clickBucket to have something more stable, without the sleep, and also check every test that use clickBucket.
| await PageObjects.common.sleep(500); | ||
| await retry.try(async () => { | ||
| const chartTypes = await retry.try( | ||
| async () => await find.allByCssSelector('li.list-group-item.list-group-menu-item')); |
There was a problem hiding this comment.
I think we need to find a better selector than this one, it's also used on the metric list. I know that we then heve a check on the name, but it's not the right way to get this.
There was a problem hiding this comment.
i agree, but would probably keep this PR to the minimal change.
| const chartString = await chart.getVisibleText(); | ||
| if (chartString === bucketName) { | ||
| await chart.click(); | ||
| await PageObjects.common.sleep(500); |
There was a problem hiding this comment.
We cannot rely on a timer to be sure that the click is executed.
There was a problem hiding this comment.
what would be your suggestion ? anyway, i would keep that out of this PR ... in general i think we want to remove every sleep in our tests with something more reliable.
💚 Build Succeeded |
|
retest |
💔 Build Failed |
|
x-pack tests failed to start, but selenium passed ok |
ppisljar
force-pushed
the
fix/flakyTestDataTable
branch
from
37fe876 to
976918d
Compare
💚 Build Succeeded |
|
@ppisljar I think it's better to create an issue for the failing build because of x-pack. I've searched this and found the same error here #20497 (comment) and there: #20111 (comment) |
|
retest |
markov00
reviewed
Aug 24, 2018
markov00
requested changes
Aug 24, 2018
There was a problem hiding this comment.
@ppisljar I forgot some files that use the directives here and I want to be sure that everything was taken in consideration:
- src/ui/public/agg_types/controls/order_agg.html
- src/ui/public/agg_types/controls/sub_agg.html
- src/ui/public/agg_types/controls/sub_metric.html
- src/ui/public/vis/editors/default/tests/agg_params.js line 84
- src/ui/public/vis/editors/default/agg.html
We have also other two visualize_page method that use the old group-name attribute:
async clickAddMetric() {
await find.clickByCssSelector('[group-name="metrics"] [data-test-subj="visualizeEditorAddAggregationButton"]');
}
async clickAddBucket() {
await find.clickByCssSelector('[group-name="buckets"] [data-test-subj="visualizeEditorAddAggregationButton"]');
}
|
I agree @markov00 |
💚 Build Succeeded |
|
retest |
💚 Build Succeeded |
|
retest |
💚 Build Succeeded |
|
retest |
markov00
approved these changes
Aug 26, 2018
💚 Build Succeeded |
|
retest |
💚 Build Succeeded |
ppisljar
added a commit
to ppisljar/kibana
that referenced
this pull request
Aug 27, 2018
ppisljar
added a commit
that referenced
this pull request
Aug 27, 2018
liza-mae
added
failed-test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes a flaky test
we didn't check if anything was actually clicked in the
clickBucketfunction