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

Replacing es_archives/reporting/ecommerce_kibana with kbn_archiver/reporting/ecommerce.json as part of migrating to kbn_archiver #102825

Merged
merged 13 commits into from
Jun 22, 2021

Conversation

bhavyarm
Copy link
Contributor

@bhavyarm bhavyarm commented Jun 21, 2021

Part of #102552

These are the steps after you find out where is the .kibana data getting loaded in the test ( sometimes you can find it by name other times look at the file)

  1. Load ecommerce_kibana data by running
NODE_TLS_REJECT_UNAUTHORIZED=0 node --no-warnings scripts/es_archiver load x-pack/test/functional/es_archives/reporting/ecommerce_kibana --config test/functional/config.js  --es-url=http://elastic:changeme@localhost:9220 --kibana-url=http://elastic:changeme@localhost:5620
  1. Find all the different types of saved objects in .kibana loaded in the first step by running
cat  x-pack/test/functional/es_archives/reporting/ecommerce_kibana/data.json | awk '{$1=$1};1' | fgrep '"type":' | fgrep -v '"type": "doc"' | sort | uniq
  1. Now save .kibana json into KBN archiver by running (please feed the types from step 2 here - dashboard,index-pattern,search,visualization )
node scripts/kbn_archiver.js --config x-pack/test/functional/config.js save x-pack/test/functional/fixtures/kbn_archiver/reporting/ecommerce.json --type dashboard,index-pattern,search,visualization
  1. Make the changes in the test file reporting.ts. Replace esArchiver loading .kibana and unloading:
await kibanaServer.importExport.load('x-pack/test/functional/fixtures/kbn_archiver/reporting/ecommerce.json');
await esArchiver.unload('x-pack/test/functional/es_archives/reporting/ecommerce');
  1. Run the test - make sure there are no errors and test runs without any issues
  2. Find which other tests are using the esArchiver from step 1 and replace it with kbnArchiver data path
  3. Delete the corresponding .kibana esArchiver file - we are replacing that with the new file from step 3
  4. Commit your changes

@bhavyarm bhavyarm self-assigned this Jun 21, 2021
@bhavyarm bhavyarm added test_xpack_functional Team:QA Team label for QA Team (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Jun 21, 2021
@bhavyarm bhavyarm changed the title kbnArchiver changes for reporting.ts Modifying reporting.ts test to work with kbnArchiver Jun 21, 2021
@bhavyarm
Copy link
Contributor Author

@elasticmachine merge upstream

@bhavyarm bhavyarm changed the title Modifying reporting.ts test to work with kbnArchiver Replacing es_archives/reporting/ecommerce_kibana with kbn_archiver/reporting/ecommerce.json as part of migrating to kbn_archiver Jun 21, 2021
@bhavyarm
Copy link
Contributor Author

@elasticmachine merge upstream

@bhavyarm
Copy link
Contributor Author

@elasticmachine merge upstream

@dmlemeshko
Copy link
Member

We got an image comparison failure:

     │      Error: expected 0.09129735815213762 to be below 0.09
     │       at Assertion.assert (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:100:11)
     │       at Assertion.lessThan.Assertion.below (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:336:8)
     │       at Function.lessThan (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:531:15)
     │       at Context.<anonymous> (test/functional/apps/dashboard/reporting/screenshots.ts:162:35)
     │       at Object.apply (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/test/src/functional_test_runner/lib/mocha/wrap_function.js:73:16)

0.09129735815213762 is something that we are getting constantly, so at least it is not a flakiness.

I checked failure screenshot and obviously there is no significant difference with the baseline, I can suggest running the test locally and see output.

@marius-dr might have other ideas

@marius-dr
Copy link
Member

Looking at the screenshots, there seem to be a few problems:

  1. the screenshot needs updating due to the font change. I think the baseline was added before last week when we got the proper font in.
  2. there's a toast with "unable to update UI setting" which shouldn't be there.
  3. The area chart seems completely different as well, maybe it was changed from Vislib to Lens.
  4. same with the donut chart.

Easy links for reference:
https://storage.googleapis.com/kibana-ci-artifacts/jobs/elastic%2Bkibana%2Bpipeline-pull-request/132986/ci-worker/parallel/20/kibana/x-pack/test/functional/screenshots/session/small_dashboard_preserve_layout_actual-session-resized.png

https://storage.googleapis.com/kibana-ci-artifacts/jobs/elastic%2Bkibana%2Bpipeline-pull-request/132986/ci-worker/parallel/20/kibana/x-pack/test/functional/screenshots/session/small_dashboard_preserve_layout_baseline-baseline-resized.png

Also 9% is a lot of difference. I'd ask the reporting team if they want to update the baseline and lower that percentage.

@bhavyarm
Copy link
Contributor Author

@elasticmachine merge upstream

@bhavyarm
Copy link
Contributor Author

Thanks @dmlemeshko @marius-dr I have skipped that test - created an issue #102911 and starting work on it.

@bhavyarm bhavyarm marked this pull request as ready for review June 22, 2021 15:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-qa (Team:QA)

@bhavyarm
Copy link
Contributor Author

cc @rashmivkulkarni

@bhavyarm
Copy link
Contributor Author

This skipped test is running without any issues in my local and I don't have ui settings error anymore - #102911 . Once I merge this - I will unskip the screenshots.ts test under dashboard run it multiple times in CI and merge it.

@bhavyarm
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @bhavyarm

@bhavyarm bhavyarm merged commit 859b453 into elastic:master Jun 22, 2021
@bhavyarm bhavyarm added auto-backport Deprecated - use backport:version if exact versions are needed and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 22, 2021
@elastic elastic deleted a comment from kibanamachine Jun 22, 2021
bhavyarm added a commit to bhavyarm/kibana that referenced this pull request Jun 23, 2021
…porting/ecommerce.json as part of migrating to kbn_archiver (elastic#102825)

# Conflicts:
#	x-pack/test/functional/es_archives/reporting/ecommerce_kibana/data.json
#	x-pack/test/functional/es_archives/reporting/ecommerce_kibana/mappings.json
bhavyarm added a commit that referenced this pull request Jun 23, 2021
…porting/ecommerce.json as part of migrating to kbn_archiver (#102825) (#103027)

# Conflicts:
#	x-pack/test/functional/es_archives/reporting/ecommerce_kibana/data.json
#	x-pack/test/functional/es_archives/reporting/ecommerce_kibana/mappings.json
@bhavyarm
Copy link
Contributor Author

@wayneseymour are these steps still valid? #102825 (comment)

Thanks!

@wayneseymour
Copy link
Member

kinda yes, kinda no as things have changed. Normally that should work, but there can be many "gotchas" that make things difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes Team:QA Team label for QA Team test_xpack_functional v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants