Skip to content

[EuiProvider / Functional tests] Check for EuiProvider Dev Warning#189018

Merged
tsullivan merged 17 commits intoelastic:mainfrom
tsullivan:eui-provider/fn-tests-detection
Aug 26, 2024
Merged

[EuiProvider / Functional tests] Check for EuiProvider Dev Warning#189018
tsullivan merged 17 commits intoelastic:mainfrom
tsullivan:eui-provider/fn-tests-detection

Conversation

@tsullivan
Copy link
Copy Markdown
Member

@tsullivan tsullivan commented Jul 23, 2024 •
edited
Loading

@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch 3 times, most recently from da43610 to 460702e Compare July 30, 2024 19:30
@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch from db664e1 to dce12de Compare August 6, 2024 23:31
@tsullivan tsullivan mentioned this pull request Aug 7, 2024
Merged
@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch from dce12de to df601cd Compare August 8, 2024 19:37
@tsullivan tsullivan changed the title [EuiProvider / Functional tests] Update navigate method to check for EuiProvider Dev Warning [EuiProvider / Functional tests] Check for EuiProvider Dev Warning Aug 8, 2024
@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch from 96222b4 to 82bb60c Compare August 9, 2024 21:40
@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch 3 times, most recently from ceb634c to 227d823 Compare August 20, 2024 00:24
@elastic elastic deleted a comment from kibana-ci Aug 20, 2024
@tsullivan tsullivan added test release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// labels Aug 20, 2024
@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch from 9c1a87b to 2cefa76 Compare August 20, 2024 17:07
@tsullivan tsullivan force-pushed the eui-provider/fn-tests-detection branch from 60742f2 to 540d733 Compare August 20, 2024 21:24
@tsullivan tsullivan marked this pull request as ready for review August 20, 2024 21:24
@tsullivan tsullivan requested review from a team as code owners August 20, 2024 21:24
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Aug 20, 2024

Pinging @elastic/appex-sharedux (Team:SharedUX)

@dmlemeshko
Copy link
Copy Markdown
Contributor

dmlemeshko commented Aug 21, 2024 •
edited
Loading

Code change LGTM.
I think it is a great check to have, time will show its stability.

It might be helpful to have a few sentences in docs section, so folks can better understand how it is integrated in functional tests design. Not sure about place, but we have dev_docs/operations/writing_stable_functional_tests.mdx

dmlemeshko
dmlemeshko reviewed Aug 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

LGTM, left a question about docs

Dosant
Dosant reviewed Aug 21, 2024
View reviewed changes
packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx Outdated
console.error(errorObject);

// 2. store error in sessionStorage so it can be detected in testing
sessionStorage.setItem('dev.euiProviderWarning.message', providerError.toString());
Copy link
Copy Markdown
Contributor

@Dosant Dosant Aug 21, 2024

Choose a reason for hiding this comment

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

tiny nit: I'd put all of this under a single key as a serialized json since they are part of the same state

Copy link
Copy Markdown
Member Author

@tsullivan tsullivan Aug 21, 2024

Choose a reason for hiding this comment

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

Good point. I have pushed 8d675a6

@tsullivan
Copy link
Copy Markdown
Member Author

tsullivan commented Aug 21, 2024

@dmlemeshko

It might be helpful to have a few sentences in docs section, so folks can better understand how it is integrated in functional tests design. Not sure about place, but we have dev_docs/operations/writing_stable_functional_tests.mdx

I agree that shining more light on this change will help developers avoid mistakes. I have pushed a5ec727

@tsullivan tsullivan requested review from Dosant and dmlemeshko August 21, 2024 20:27
dmlemeshko
dmlemeshko approved these changes Aug 22, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

LGTM. Great work!

Dosant
Dosant approved these changes Aug 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Code lgtm 👍

@tsullivan tsullivan enabled auto-merge (squash) August 26, 2024 18:20
@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Aug 26, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 420.2KB 420.4KB +214.0B

History

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

@tsullivan tsullivan merged commit dc7e3ec into elastic:main Aug 26, 2024
@kibanamachine kibanamachine added v8.16.0 backport:skip This PR does not require backporting labels Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Dosant Dosant Dosant approved these changes

@dmlemeshko dmlemeshko dmlemeshko approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// test v8.16.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tsullivan @elasticmachine @dmlemeshko @kibana-ci @Dosant @kibanamachine