Skip to content

Brittle test order in Karma - shared <body> and data-page attribute usage

Some of our karma tests are failing when run in focus mode. For example any tests in this block.

All the tests seem to pass, if you run without any fdescribe/fit and the whole 2k+ test suite runs. But when I add some focus blocks to the tests in question, the test fails because the spy doesn't actually stub getPagePath or we can't find the data-page attribute in the fixture markup (which doesn't exist and why we used a spy).

Chrome MergeRequestTabs loadDiff requires an absolute pathname FAILED
	TypeError: Cannot read property 'split' of undefined
	    at getPagePath (webpack:///lib/utils/common_utils.js:7:0 <- spec/javascripts/test_bundle.js:272023:32)
	    at new Notes (webpack:///notes.js:93:0 <- spec/javascripts/test_bundle.js:278768:115)
	    at Object.<anonymous> (webpack:///Users/eric/Documents/gitlab/gitlab-development-kit/gitlab/spec/javascripts/merge_request_tabs_spec.js:279:0 <- spec/javascripts/test_bundle.js:186883:24)

This is caused by our move to modulify'ing common_utils.js because notes.js now imports getPagePath directly as a module instead of relying on the window.gl.utils.getPagePath global which the spy in the test is targeting.

Other tests, stub this differently by adding the data-page attribute onto whatever <body> is available,

beforeEach(() => {
  $('body').data('page', 'projects:merge_requests:show');
});

What confused me is how the tests pass when the whole test suite is ran 🤔

Logging out the markup shows that the data-page="projects:merge_requests:show" attribute is available on the <body>. But what is setting it 🤔

Previous tests! 😧 😨

Solutions

As a overarching goal, we probably want to get rid of the brittle test order relationship when setting any attribute on the <body> itself as it looks to be shared across tests (although convenient). This could be solved by fixing karma to not share any fixture markup, or cleaning up any <body> modification in an after hook which is dubious.

Also in order to spy and stub things appropriately that get imported by a module, we probably want to come to a conclusion on https://gitlab.com/gitlab-org/gitlab-ce/issues/30998

As a quick-fix for the tests in hand we could add the data-page modification.

beforeEach(() => {
  $('body').data('page', 'projects:merge_requests:show');
});

cc @gl-frontend

Edited by Eric Eastwood