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

[SharedUX] Fix warning in solution_nav test #129322

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

majagrubic
Copy link
Contributor

Summary

This PR fixes the following warning in the solution_nav test.

    Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined

The problem was that @elastic/eui library is mocked, but only one method was actually implemented. The PR adds a mock implementation of EuiSideNav as well.

Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

  • Unit or functional tests were updated or added to match the most common scenarios
    - [ ] Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
    - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
    - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
    - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
    - [ ] This was checked for cross-browser compatibility

For maintainers

@majagrubic majagrubic requested a review from a team April 4, 2022 11:13
@majagrubic majagrubic added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) release_note:skip Skip the PR/issue when compiling release notes v8.3.0 labels Apr 4, 2022
@@ -14,6 +14,9 @@ jest.mock('@elastic/eui', () => ({
useIsWithinBreakpoints: (args: string[]) => {
return args[0] === 'xs';
},
EuiSideNav: function Component() {
// no-op
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't a functional component always return? It seems that for consistency we should return null. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think when returning null we'd explicitly want it to be null. By "no-op" we're saying "I don't care what this returns".

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #10 / Actions and Triggers app Rule Details Header should reenable a disabled the rule

Metrics [docs]

✅ unchanged

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

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

Approving as my comment is a question and should not block merging. 👍

@majagrubic majagrubic merged commit 3fd4621 into elastic:main Apr 4, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 4, 2022
@majagrubic majagrubic deleted the fix-test-warning branch April 4, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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) v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants