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

Added alerting ui mock for jest test #92604

Merged
merged 7 commits into from
Mar 1, 2021
Merged

Added alerting ui mock for jest test #92604

merged 7 commits into from
Mar 1, 2021

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Feb 24, 2021 •
edited
Loading

Summary

Needed a mock as can be seen in example PR #91469 to test jest test

so added a mock for alerting UI

Note:

Need to check if this will have any adverse effect on page load bundle size

Update

ended up improving lazy loading in alerting plugin to offset cost

@shahzad31 shahzad31 requested a review from a team as a code owner February 24, 2021 11:59
@shahzad31 shahzad31 changed the title [Uptime] added alerting ui mock for jest test Added alerting ui mock for jest test Feb 24, 2021
@shahzad31 shahzad31 self-assigned this Feb 24, 2021
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0 labels Feb 24, 2021
mikecote
mikecote reviewed Feb 24, 2021
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the PR!

Could we make the following changes:

  • Rename mock.ts to mocks.ts
  • Use factory pattern for consistency (example: x-pack/plugins/alerts/server/mocks.ts).
  • Should be able to access the start mock via something like triggersActionsUiMock.createStart()

@shahzad31
Copy link
Contributor Author

Add 3.6kb into the page load bundle , i will try to offset with some lazy loading

image

@mikecote
Copy link
Contributor

@shahzad31 feel free to remove the changes in x-pack/plugins/triggers_actions_ui/public/index.ts to reduce bundle size. I've seen a pattern where it's ok to require mocks directly via /plugin/server/mocks.ts.

@shahzad31
Copy link
Contributor Author

@mikecote ended up spending an hour improving lazy loading in the app. i hope it helps.

@shahzad31 shahzad31 requested a review from a team as a code owner February 25, 2021 08:28
@shahzad31
Copy link
Contributor Author

updated infra paths to use lazy components

YulNaumenko
YulNaumenko approved these changes Feb 25, 2021
View reviewed changes
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1.9MB 1.9MB -97.0B
triggersActionsUi 1.4MB 1.5MB +121.0KB
total +120.9KB

Page load bundle

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

id before after diff
infra 166.3KB 166.0KB -261.0B
triggersActionsUi 147.8KB 104.1KB -43.7KB
total -43.9KB
Unknown metric groups

async chunk count

id before after diff
triggersActionsUi 32 42 +10

History

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

cc @shahzad31

Kerry350
Kerry350 approved these changes Mar 1, 2021
View reviewed changes
Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

LGTM for infra 👍

@shahzad31 shahzad31 merged commit b3499f0 into elastic:master Mar 1, 2021
@shahzad31 shahzad31 deleted the alert-ui-mock branch March 1, 2021 11:55
@shahzad31 shahzad31 added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 1, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 1, 2021
@shahzad31 @kibanamachine
Added alerting ui mock for jest test (elastic#92604)
bb2a152 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine mentioned this pull request Mar 1, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #93020

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 1, 2021
@kibanamachine @shahzad31
Added alerting ui mock for jest test (#92604) (#93020)
9a97394 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Shahzad <shahzad.muhammad@elastic.co>
jloleysens added a commit that referenced this pull request Mar 3, 2021
@jloleysens
Merge branch 'ilm/rollup-v2-action' of github.com:elastic/kibana into…
cae0462 
… ilm/rollup-v2-action

* 'ilm/rollup-v2-action' of github.com:elastic/kibana:
  [Security Solution][Case][Bug] Only add rule object for alert comments (#92977)
  [Security Solution][Case] Show the current connector name in case view (#93018)
  [Security Solution] Remove unused mock data (#92357)
  Adds mapping to the signals for the indicator rules that were missing (#92928)
  skip flaky suite (#85208)
  Cleanup spaces plugin (#91976)
  Control round and decimal places in Gauge Visualization when using aggregate functions like average (#91293)
  Added alerting ui mock for jest test (#92604)
  Remove "beta" label from URL Drilldown as it is now GA (#92859)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikecote mikecote mikecote left review comments

@Kerry350 Kerry350 Kerry350 approved these changes

@YulNaumenko YulNaumenko YulNaumenko approved these changes

Assignees

@shahzad31 shahzad31

Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@shahzad31 @mikecote @kibanamachine @Kerry350 @YulNaumenko