[APM] Fix broken unit tests #161636

sorenlouv · 2023-07-11T10:40:10Z

A bunch of APM unit tests were passing on CI but failing locally. This PR fixes the unit tests

Why fail locally and pass on CI??
The reason they pass on CI is because console.* is disabled on CI:

kibana/packages/kbn-test/src/jest/setup/disable_console_logs.js

Lines 9 to 12 in a78c7b0

    
           // on CI these logs just muddy up the console and produce a ton of unnecessary noise 
        
           console.log = () => {}; 
        
           console.error = () => {}; 
        
           console.warn = () => {};

Whereas in the APM jest config console.error is treated as a test failure:

kibana/x-pack/plugins/apm/jest_setup.js

Lines 12 to 15 in 7ea0dd6

    
           jest.spyOn(console, 'error').mockImplementation((message, ...args) => { 
        
             console.log(message, ...args); 
        
             throw new Error(message); 
        
           });

elasticmachine · 2023-07-11T10:40:17Z

Pinging @elastic/apm-ui (Team:APM)

apmmachine · 2023-07-11T10:40:25Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

kibana-ci · 2023-07-11T12:04:10Z

💛 Build succeeded, but was flaky

Failed CI Steps

APM Cypress Tests #3

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	14	16	+2
`securitySolution`	409	413	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	15	17	+2
`securitySolution`	488	492	+4
total			+6

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

gbamparop · 2023-07-11T15:41:16Z

Thanks for fixing this! Can we have the configs aligned to prevent this from happening in the future?

sorenlouv · 2023-07-12T12:00:06Z

Can we have the configs aligned to prevent this from happening in the future?

Maybe. console.* is disabled globally on CI so we'd either have to enable it for everyone or find a way to only enable it for APM:

kibana/packages/kbn-test/src/jest/setup/disable_console_logs.js

Lines 9 to 12 in a78c7b0

    
           // on CI these logs just muddy up the console and produce a ton of unnecessary noise 
        
           console.log = () => {}; 
        
           console.error = () => {}; 
        
           console.warn = () => {};

Either way, I don't think that should block this PR.

gbamparop · 2023-07-12T15:18:44Z

Can we have the configs aligned to prevent this from happening in the future?

Maybe. console.* is disabled globally on CI so we'd either have to enable it for everyone or find a way to only enable it for APM:

kibana/packages/kbn-test/src/jest/setup/disable_console_logs.js

Lines 9 to 12 in a78c7b0

// on CI these logs just muddy up the console and produce a ton of unnecessary noise

console.log = () => {};

console.error = () => {};

console.warn = () => {};

Either way, I don't think that should block this PR.

Agree, shouldn't block the PR. An alternative is to remove our difference in the jest config.

gbamparop

LGTM!

A bunch of APM unit tests were passing on CI but failing locally. This PR fixes the unit tests **Why fail locally and pass on CI??** The reason they pass on CI is because `console.error` calls are omitted. In the APM jest config `console.error` is treated as a test failure: https://github.com/elastic/kibana/blob/7ea0dd6b116a93024d68ea2d93fa4ce90e9bf189/x-pack/plugins/apm/jest_setup.js#L12-L15 (cherry picked from commit ccb36d9)

kibanamachine · 2023-07-12T15:24:37Z

💚 All backports created successfully

Status	Branch	Result
✅	8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.9`: - [[APM] Fix broken unit tests (#161636)](#161636)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Søren Louv-Jansen <[email protected]>

#161636 fixed a couple of unit tests that were failing locally but passing on CI. This PR should prevent this from happening again. **Why they failed locally and not on CI??** Locally `console.error` is treated as a test failure: https://github.com/elastic/kibana/blob/7ea0dd6b116a93024d68ea2d93fa4ce90e9bf189/x-pack/plugins/apm/jest_setup.js#L12-L15 Whereas on CI `console.*` is disabled: https://github.com/elastic/kibana/blob/a78c7b02b3b825826f39289e91e545ee6f4a67d9/packages/kbn-test/src/jest/setup/disable_console_logs.js#L9-L12 This means that if a test logs `console.error` it would fail locally but not on CI. This PR changes that so console.error will not cause unit tests to fail anywhere.

…ic#161959) elastic#161636 fixed a couple of unit tests that were failing locally but passing on CI. This PR should prevent this from happening again. **Why they failed locally and not on CI??** Locally `console.error` is treated as a test failure: https://github.com/elastic/kibana/blob/7ea0dd6b116a93024d68ea2d93fa4ce90e9bf189/x-pack/plugins/apm/jest_setup.js#L12-L15 Whereas on CI `console.*` is disabled: https://github.com/elastic/kibana/blob/a78c7b02b3b825826f39289e91e545ee6f4a67d9/packages/kbn-test/src/jest/setup/disable_console_logs.js#L9-L12 This means that if a test logs `console.error` it would fail locally but not on CI. This PR changes that so console.error will not cause unit tests to fail anywhere. (cherry picked from commit c842479)

…161959) (#162100) # Backport This will backport the following commits from `main` to `8.9`: - [[APM] Prevent console.error causing unit tests to fail locally (#161959)](#161959)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Søren Louv-Jansen <[email protected]>

…ic#161959) elastic#161636 fixed a couple of unit tests that were failing locally but passing on CI. This PR should prevent this from happening again. **Why they failed locally and not on CI??** Locally `console.error` is treated as a test failure: https://github.com/elastic/kibana/blob/7ea0dd6b116a93024d68ea2d93fa4ce90e9bf189/x-pack/plugins/apm/jest_setup.js#L12-L15 Whereas on CI `console.*` is disabled: https://github.com/elastic/kibana/blob/a78c7b02b3b825826f39289e91e545ee6f4a67d9/packages/kbn-test/src/jest/setup/disable_console_logs.js#L9-L12 This means that if a test logs `console.error` it would fail locally but not on CI. This PR changes that so console.error will not cause unit tests to fail anywhere.

[APM] Fix broken unit tests

53c6e9a

sorenlouv requested a review from a team as a code owner July 11, 2023 10:40

botelastic bot added the Team:APM All issues that need APM UI Team support label Jul 11, 2023

sorenlouv added v8.9.0 release_note:skip Skip the PR/issue when compiling release notes labels Jul 11, 2023

sorenlouv enabled auto-merge (squash) July 12, 2023 11:53

gbamparop approved these changes Jul 12, 2023

View reviewed changes

sorenlouv merged commit ccb36d9 into elastic:main Jul 12, 2023

kibanamachine added the v8.10.0 label Jul 12, 2023

kibanamachine mentioned this pull request Jul 12, 2023

[8.9] [APM] Fix broken unit tests (#161636) #161782

Merged

sorenlouv deleted the fix-broken-unit-tests branch July 13, 2023 12:13

sorenlouv mentioned this pull request Jul 14, 2023

[APM] Prevent console.error causing unit tests to fail locally #161959

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Fix broken unit tests #161636

[APM] Fix broken unit tests #161636

sorenlouv commented Jul 11, 2023 •

edited by kibanamachine

Loading

elasticmachine commented Jul 11, 2023

apmmachine commented Jul 11, 2023

kibana-ci commented Jul 11, 2023

ESLint disabled line counts

Total ESLint disabled count

gbamparop commented Jul 11, 2023

sorenlouv commented Jul 12, 2023 •

edited

Loading

gbamparop commented Jul 12, 2023

gbamparop left a comment

kibanamachine commented Jul 12, 2023

	// on CI these logs just muddy up the console and produce a ton of unnecessary noise
	console.log = () => {};
	console.error = () => {};
	console.warn = () => {};

	jest.spyOn(console, 'error').mockImplementation((message, ...args) => {
	console.log(message, ...args);
	throw new Error(message);
	});

[APM] Fix broken unit tests #161636

[APM] Fix broken unit tests #161636

Conversation

sorenlouv commented Jul 11, 2023 • edited by kibanamachine Loading

elasticmachine commented Jul 11, 2023

apmmachine commented Jul 11, 2023

🤖 GitHub comments

kibana-ci commented Jul 11, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

gbamparop commented Jul 11, 2023

sorenlouv commented Jul 12, 2023 • edited Loading

gbamparop commented Jul 12, 2023

gbamparop left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 12, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Jul 11, 2023 •

edited by kibanamachine

Loading

sorenlouv commented Jul 12, 2023 •

edited

Loading