[Uptime] Remove prototype overwrite in unit test #73709

justinkambic · 2020-07-29T16:39:19Z

Summary

Follow-up to #73531, we are looking to avoid directly overwriting the prototype of moment in a test file. I have also wrapped a few other direct prototype overwrites with jest.spyOn.

elasticmachine · 2020-07-29T16:39:22Z

Pinging @elastic/uptime (Team:uptime)

shahzad31 · 2020-07-29T16:47:38Z

x-pack/plugins/uptime/public/components/common/charts/__tests__/ping_histogram.test.tsx

+jest.mock('moment-timezone', () => {
+  return function () {
+    return { tz: { guess: () => 'America/New_York' } };
+  };
+});
+
+jest.mock('moment', () => {
+  return function () {
+    return { fromNow: () => 'a year ago' };
+  };
+});
+
+jest.mock('../../../../../../../../src/plugins/data/public', () => {
+  return function () {
+    return {
+      esKuery: {
+        fromKueryExpression: () => 'an ast',
+        toElasticsearchQuery: () => 'an es query',
+      },
+    };
+  };
+});


can we move these mocks to a test_util file, so that these can also be used in other places.

Do you mean the custom module factories? So use would look like:

import { momentMockFactory } from './util'; jest.mock('moment', momentMockFactory);

We could modularize them, but they're kind of specific to the needs of this test file. moment-timezone probably won't need to be mocked in most situations, for example, and there are lots of cases where we'll need more than just fromNow on moment mocks.

can we consolidate all these mocks, so we can use it in other places as well https://github.com/elastic/kibana/blob/master/x-pack/plugins/uptime/public/lib/helper/test_helpers.ts

…tions

…th-new-testing-utils

shahzad31 · 2021-01-12T12:45:02Z

x-pack/plugins/uptime/public/components/common/charts/ping_histogram.test.tsx

+    mockMoment();
+    mockMomentTimezone();
+    mockDataPlugin();


instead of running these in each test, do you think we should move these as part of jest setup?

in uptime jest.config.js?

We should do this in a follow-up: elastic/uptime#283

shahzad31 · 2021-01-12T14:56:01Z

x-pack/plugins/uptime/public/lib/helper/component_test_helpers.tsx

@@ -0,0 +1,187 @@
+/*


i think file needs to be removed.

yeah it was my mistake in a previous merge, I went in and got rid of it.

justinkambic · 2021-01-12T15:01:34Z

x-pack/plugins/uptime/public/components/common/charts/ping_histogram.test.tsx

+    mockMoment();
+    mockMomentTimezone();
+    mockDataPlugin();


We should do this in a follow-up: elastic/uptime#283

justinkambic · 2021-01-12T15:03:00Z

x-pack/plugins/uptime/public/components/common/charts/ping_histogram.test.tsx

@@ -48,18 +58,13 @@ describe('PingHistogram component', () => {
    },
  };

-  it('shallow renders the component without errors', () => {


I decided this test wasn't worth keeping. It asserted that the PingHistogramComponent contained the props that were passed to it, and not much else.

Rendering these components doesn't yield SVG or any other method of ensuring the data is rendered by Elastic Charts, but we also should not be testing those capabilities in our repo.

shahzad31 · 2021-01-12T15:14:18Z

...lugins/uptime/public/components/overview/monitor_list/columns/monitor_status_column.test.tsx

+
+    fireEvent.mouseOver(locationsContainer);
+
+    await waitFor(() => screen.getByText('Up in BerlinDown in Islamabad, st-paul'));


Should there be a space between?

Good question, I'll have a look in a real-world case. It could just be the way the raw html looks.

It was indeed a bug; we could've tackled it in a separate issue but I fixed it in 2a22f29.

shahzad31

LGTM !!

justinkambic · 2021-01-12T17:09:18Z

@shahzad31 can you please have a look at 2a22f29 and a98b048 to make sure you're still 👍 on this.

shahzad31 · 2021-01-12T17:12:07Z

@shahzad31 can you please have a look at 2a22f29 and a98b048 to make sure you're still 👍 on this.

Looks good !!

kibanamachine · 2021-01-12T19:14:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a98b048

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
`uptime`	1.1MB	1.1MB	+353.0B

History

💚 Build #98246 succeeded 98bdfe0
💔 Build #98203 failed f143036
💚 Build #97730 succeeded aa70db5
💔 Build #97667 failed 0b5f791
💔 Build #97654 failed 6501666

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

* Remove prototype overwrite in unit test. * Wrap calls to prototype functions with jest.spyOn. * add additional component test helpers * add test examples * uptime testing utils remove custom prefix from props and parameter options * skip executed step tests * adjust MlJobLink test * add testing util interfaces * Move updated test files to correct locations. * Move other test file to correct location. * Revert unintended changes from merge. * Revert unintended changes from merge. * update mock core * combine wrappers into one custom render function * Move mock helpers to helper file. Update snapshots. * Refactor tests to use RTL over enzyme. * Refactor \`PingHistogram\` component tests to prefer RTL to Enzyme. * Drop obsolete snapshot file. * Remove obsolete file leftover from merge error. * Fix outdated import path. * Prefer custom render to vanilla. * Fix formatting issue uncovered by unit test, and update test assertion. * Add test for single location. Co-authored-by: Dominique Clarke <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

* Remove prototype overwrite in unit test. * Wrap calls to prototype functions with jest.spyOn. * add additional component test helpers * add test examples * uptime testing utils remove custom prefix from props and parameter options * skip executed step tests * adjust MlJobLink test * add testing util interfaces * Move updated test files to correct locations. * Move other test file to correct location. * Revert unintended changes from merge. * Revert unintended changes from merge. * update mock core * combine wrappers into one custom render function * Move mock helpers to helper file. Update snapshots. * Refactor tests to use RTL over enzyme. * Refactor \`PingHistogram\` component tests to prefer RTL to Enzyme. * Drop obsolete snapshot file. * Remove obsolete file leftover from merge error. * Fix outdated import path. * Prefer custom render to vanilla. * Fix formatting issue uncovered by unit test, and update test assertion. * Add test for single location. Co-authored-by: Dominique Clarke <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Dominique Clarke <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

justinkambic · 2021-01-13T12:44:45Z

Backported to:

7.x/7.12.0 8dbe468
[7.x] [Uptime] Remove prototype overwrite in unit test (#73709) #88090
7.11/7.11.0 f497199
[7.11] [Uptime] Remove prototype overwrite in unit test (#73709) #88091

justinkambic added 2 commits July 29, 2020 12:14

Remove prototype overwrite in unit test.

ef05965

Wrap calls to prototype functions with jest.spyOn.

56fed24

justinkambic added v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes test-unit v7.10.0 7.9.0 labels Jul 29, 2020

justinkambic requested review from tylersmalley and shahzad31 July 29, 2020 16:39

justinkambic requested a review from a team as a code owner July 29, 2020 16:39

justinkambic self-assigned this Jul 29, 2020

shahzad31 reviewed Jul 29, 2020

View reviewed changes

lukasolson added v7.9.0 and removed 7.9.0 labels Jul 29, 2020

Merge branch 'master' into uptime_ping-histogram-jest-mock-moment

15cfb3b

spalger added v7.9.2 and removed v7.9.0 labels Sep 3, 2020

LeeDr added v7.9.3 and removed v7.9.2 labels Sep 18, 2020

justinkambic removed v7.10.0 v7.9.3 labels Oct 12, 2020

tylersmalley approved these changes Jan 6, 2021

View reviewed changes

dominiqueclarke added 6 commits January 7, 2021 10:02

add additional component test helpers

69628a5

add test examples

afea632

uptime testing utils remove custom prefix from props and parameter op…

0ec06b0

…tions

skip executed step tests

97d57a6

adjust MlJobLink test

3294340

add testing util interfaces

05b0b6b

dominiqueclarke and others added 8 commits January 7, 2021 15:38

combine wrappers into one custom render function

48f1536

Move mock helpers to helper file. Update snapshots.

4f57662

Refactor tests to use RTL over enzyme.

aa70db5

Merge branch 'master' into tests/uptime-testing-utils

5ade268

Merge branch 'tests/uptime-testing-utils' into prototype-overwrite-wi…

6a2cc03

…th-new-testing-utils

Refactor \PingHistogram\ component tests to prefer RTL to Enzyme.

fd10c59

Drop obsolete snapshot file.

c8d188f

Merge branch 'master' into prototype-overwrite-with-new-testing-utils

f143036

shahzad31 reviewed Jan 12, 2021

View reviewed changes

justinkambic added 2 commits January 12, 2021 07:57

Remove obsolete file leftover from merge error.

50949ec

Fix outdated import path.

f7ba9eb

shahzad31 reviewed Jan 12, 2021

View reviewed changes

Prefer custom render to vanilla.

98bdfe0

justinkambic commented Jan 12, 2021

View reviewed changes

shahzad31 reviewed Jan 12, 2021

View reviewed changes

shahzad31 approved these changes Jan 12, 2021

View reviewed changes

justinkambic added 2 commits January 12, 2021 11:58

Fix formatting issue uncovered by unit test, and update test assertion.

2a22f29

Add test for single location.

a98b048

justinkambic merged commit 1094274 into elastic:master Jan 12, 2021

This was referenced Jan 12, 2021

[7.x] [Uptime] Remove prototype overwrite in unit test (#73709) #88090

Merged

[7.11] [Uptime] Remove prototype overwrite in unit test (#73709) #88091

Merged

justinkambic deleted the uptime_ping-histogram-jest-mock-moment branch January 13, 2021 12:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Uptime] Remove prototype overwrite in unit test #73709

[Uptime] Remove prototype overwrite in unit test #73709

justinkambic commented Jul 29, 2020

elasticmachine commented Jul 29, 2020

shahzad31 Jul 29, 2020

justinkambic Jul 29, 2020

shahzad31 Jan 7, 2021

shahzad31 Jan 12, 2021

justinkambic Jan 12, 2021

shahzad31 Jan 12, 2021

justinkambic Jan 12, 2021

justinkambic Jan 12, 2021

justinkambic Jan 12, 2021

shahzad31 Jan 12, 2021

justinkambic Jan 12, 2021

justinkambic Jan 12, 2021

shahzad31 left a comment

justinkambic commented Jan 12, 2021

shahzad31 commented Jan 12, 2021

kibanamachine commented Jan 12, 2021

justinkambic commented Jan 13, 2021


		fireEvent.mouseOver(locationsContainer);

		await waitFor(() => screen.getByText('Up in BerlinDown in Islamabad, st-paul'));

[Uptime] Remove prototype overwrite in unit test #73709

[Uptime] Remove prototype overwrite in unit test #73709

Conversation

justinkambic commented Jul 29, 2020

Summary

elasticmachine commented Jul 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahzad31 left a comment

Choose a reason for hiding this comment

justinkambic commented Jan 12, 2021

shahzad31 commented Jan 12, 2021

kibanamachine commented Jan 12, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

justinkambic commented Jan 13, 2021