Skip to content

Added alerting ui mock for jest test#92604

Merged
shahzad31 merged 7 commits intoelastic:masterfrom
shahzad31:alert-ui-mock
Mar 1, 2021
Merged

Added alerting ui mock for jest test#92604
shahzad31 merged 7 commits intoelastic:masterfrom
shahzad31:alert-ui-mock

Conversation

@shahzad31
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 commented Feb 24, 2021

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
Copy link
Copy Markdown
Contributor

@mikecote mikecote left a comment

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
Copy Markdown
Contributor Author

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

image

@mikecote
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

updated infra paths to use lazy components

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
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

Copy link
Copy Markdown
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
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Copy Markdown
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
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
… 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

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

Development

Successfully merging this pull request may close these issues.

5 participants