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.

Sign up for GitHub

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

Jump to bottom

[Tests] update expected value for percentile ranks #1822

Merged
merged 3 commits into from
Jun 30, 2022
Merged

[Tests] update expected value for percentile ranks #1822

merged 3 commits into from
Jun 30, 2022

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Jun 29, 2022 •
edited
Loading

Description

Origin:
opensearch-project/OpenSearch#3634

The previous value was actually incorrect after OpenSearch bumped t-digest
the value is now the correct value.

Signed-off-by: Kawika Avilla kavilla414@gmail.com

Issues Resolved

#1821

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

@kavilla kavilla requested a review from a team as a code owner June 29, 2022 21:34
manasvinibs
manasvinibs previously approved these changes Jun 29, 2022
View reviewed changes
Copy link
Member

@manasvinibs manasvinibs left a comment

Choose a reason for hiding this comment

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

Thanks @kavilla for the fix! LGTM

mihirsoni
mihirsoni previously approved these changes Jun 29, 2022
View reviewed changes
joshuarrrr
joshuarrrr previously approved these changes Jun 29, 2022
View reviewed changes
@joshuarrrr
Copy link
Member

Is this a 3.0 only change, or will we backport?

@kaddy645
Copy link
Contributor

kaddy645 commented Jun 29, 2022 •
edited
Loading

Looks like one more was failing yesterday ! metric chart should show Percentiles: should we also update values for it ?

OpenSearch-Dashboards/test/functional/apps/visualize/_metric_chart.js

Line 188 in 4ea645e

expect(percentileRankBytes).to.eql(metricValue);

@kavilla kavilla dismissed stale reviews from joshuarrrr, mihirsoni, and manasvinibs via 90b9364 June 29, 2022 22:06
@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2022 •
edited
Loading

Codecov Report

Merging #1822 (4590a93) into main (e7362f9) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1822   +/-   ##
=======================================
  Coverage   67.50%   67.51%           
=======================================
  Files        3074     3074           
  Lines       59127    59127           
  Branches     8988     8988           
=======================================
+ Hits        39915    39917    +2     
+ Misses      17028    17027    -1     
+ Partials     2184     2183    -1
Impacted Files Coverage Δ
...ic/application/models/sense_editor/sense_editor.ts 64.88% <0.00%> (+0.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7362f9...4590a93. Read the comment docs.

@kavilla kavilla force-pushed the avillk/percentiles_test_failure branch from 90b9364 to 130e3eb Compare June 29, 2022 22:33
@kavilla
Copy link
Member Author

kavilla commented Jun 29, 2022

Looks like one more was failing yesterday ! metric chart should show Percentiles: should we also update values for it ?

OpenSearch-Dashboards/test/functional/apps/visualize/_metric_chart.js

Line 188 in 4ea645e

expect(percentileRankBytes).to.eql(metricValue);

Updated!

@kavilla
Copy link
Member Author

kavilla commented Jun 29, 2022

Is this a 3.0 only change, or will we backport?

This is expected to only be a 3.0 change since this was not backported: opensearch-project/OpenSearch#3634

@kaddy645
Copy link
Contributor

kaddy645 commented Jun 29, 2022 •
edited
Loading

it might fail again! I see more missing values. can we run/test these functional tests locally ?

expected

 [ '2,147,483,648',
               │   '1st percentile of machine.ram',
               │   '3,221,225,472',
               │   '5th percentile of machine.ram',
               │   '7,516,192,768',
               │   '25th percentile of machine.ram',
               │   '12,884,901,888',
               │   '50th percentile of machine.ram',
               │   '18,253,611,008',
               │   '75th percentile of machine.ram',
               │   '32,212,254,720',
               │   '95th percentile of machine.ram',
               │   '32,212,254,720',
               │   '99th percentile of machine.ram' ] 
               
              
                '2,147,483,648',
               │   '1st percentile of machine.ram',
               │   '3,221,225,472',
               │   '5th percentile of machine.ram',
               │   '7,516,192,768',
               │   '25th percentile of machine.ram',
               │   '12,884,901,888',
               │   '50th percentile of machine.ram',
               │   '18,179,771,104.788',
               │   '75th percentile of machine.ram',
               │   '31,704,016,923.307',
               │   '95th percentile of machine.ram',
               │   '32,212,254,720',
               │   '99th percentile of machine.ram' ]

@kavilla
[Tests] update expected value for percentile ranks
3c06704 
Origin:
opensearch-project/OpenSearch#3634

The previous value was actually incorrect after OpenSearch bumped t-digest
the value is now the correct value.

Issue:
opensearch-project#1821

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla force-pushed the avillk/percentiles_test_failure branch from 130e3eb to 3c06704 Compare June 29, 2022 23:08
@kavilla
Copy link
Member Author

kavilla commented Jun 30, 2022

it might fail again! I see more missing values. can we run/test these functional tests locally ?

expected

 [ '2,147,483,648',
               │   '1st percentile of machine.ram',
               │   '3,221,225,472',
               │   '5th percentile of machine.ram',
               │   '7,516,192,768',
               │   '25th percentile of machine.ram',
               │   '12,884,901,888',
               │   '50th percentile of machine.ram',
               │   '18,253,611,008',
               │   '75th percentile of machine.ram',
               │   '32,212,254,720',
               │   '95th percentile of machine.ram',
               │   '32,212,254,720',
               │   '99th percentile of machine.ram' ] 
               
              
                '2,147,483,648',
               │   '1st percentile of machine.ram',
               │   '3,221,225,472',
               │   '5th percentile of machine.ram',
               │   '7,516,192,768',
               │   '25th percentile of machine.ram',
               │   '12,884,901,888',
               │   '50th percentile of machine.ram',
               │   '18,179,771,104.788',
               │   '75th percentile of machine.ram',
               │   '31,704,016,923.307',
               │   '95th percentile of machine.ram',
               │   '32,212,254,720',
               │   '99th percentile of machine.ram' ]

Yeah I might have to skip this test now and re-open the issue on OpenSearch. The values returned are flaky.

@kavilla
skip inconsistent values
0c09771 
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla
use slice
4590a93 
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla merged commit a9984f6 into opensearch-project:main Jun 30, 2022
@kavilla kavilla linked an issue Jul 1, 2022 that may be closed by this pull request
[BUG][Tests] should show Percentile Ranks test failure #1821
Closed
@kavilla kavilla mentioned this pull request Jul 5, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshuarrrr joshuarrrr joshuarrrr approved these changes

@kaddy645 kaddy645 kaddy645 approved these changes

@mihirsoni mihirsoni Awaiting requested review from mihirsoni

@manasvinibs manasvinibs Awaiting requested review from manasvinibs manasvinibs is a code owner

Assignees
No one assigned
Labels
test:functional v3.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG][Tests] should show Percentile Ranks test failure
6 participants
@kavilla @joshuarrrr @kaddy645 @codecov-commenter @mihirsoni @manasvinibs