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

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

Merged
merged 17 commits into from
Aug 26, 2024
Merged

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

merged 17 commits into from
Aug 26, 2024

Conversation

tsullivan
Copy link
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 Team label for AppEx-SharedUX (formerly Global Experience) 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
Contributor

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

@dmlemeshko
Copy link
Member

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
Member

@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
// eslint-disable-next-line no-console
console.error(errorObject);

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

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
Member Author

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
Member Author

@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
Member

@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
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
Collaborator

💛 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
39 checks passed
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit 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

@dmlemeshko dmlemeshko dmlemeshko approved these changes

@Dosant Dosant Dosant approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) 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