Skip to content

Make "jest minimal" CI jobs smarter with an "HTML->JS mapping"

David Dieulivol requested to merge 386719-html_to_js_mappings into master

Context

Closes #386719 (closed)

We currently are using the --findRelatedTests flag for jest by passing the files that got changed in the MR (via the RSPEC_CHANGED_FILES_PATH variable).

Jest will only find matching JS tests if we have changed JS files in the MR. If we are changing HTML files, Jest will not detect any JS tests.

What does this MR do?

We add intelligence to the jest minimal jobs, so that they run JS tests that could be related to a view change.

If files were changed inside the app/views directory, we'll try to find matching JS files in the app/assets/javascripts folder, and pass those files to jest via the --findRelatedTests flag. Jest will then find matching JS specs to run, increasing the chance of us detecting an issue.

How do we detect a mapping?

We are looking for the js-<xxx> string in the file. Those are generally used as HTML ids and classes, and are reused in JS files.

Bonus: we also have the convention to use js-vue-<xxx> HTML ids and classes, which means we're also able to propose VueJS files related to an HTML change :rocket.

Results

As a test in this MR: we reintroduced the changes made in the MR that provoked the previously mentioned broken master: !91954 (diffs).

detect-tests

https://gitlab.com/gitlab-org/gitlab/-/jobs/3547070752#L178:

Related JS files: 

app/assets/javascripts/awards_handler.js app/assets/javascripts/pages/projects/snippets/show/index.js
app/assets/javascripts/pages/projects/merge_requests/init_merge_request_show.js
app/assets/javascripts/notes/stores/actions.js
app/assets/javascripts/vue_shared/components/awards_list.vue
app/assets/javascripts/issues/index.js
app/assets/javascripts/deprecated_notes.js

js minimal failing job

https://gitlab.com/gitlab-org/gitlab/-/jobs/3547071279:

FAIL spec/frontend/awards_handler_spec.js
  ● AwardsHandler › ::removeYouToUserList › removes "You" from the front of the tooltip
    expect(received).toBe(expected) // Object.is equality
    Expected: "sam, jerry, max, and andy"
    Received: undefined
      253 |       awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
      254 |
    > 255 |       expect($thumbsUpEmoji.attr('title')).toBe('sam, jerry, max, and andy');
          |                                            ^
      256 |     });
      257 |
      258 |     it('handles the special case where "You" is not cleanly comma separated', () => {
      at Object.toBe (spec/frontend/awards_handler_spec.js:255:44)
  ● AwardsHandler › ::removeYouToUserList › handles the special case where "You" is not cleanly comma separated
    expect(received).toBe(expected) // Object.is equality
    Expected: "sam"
    Received: undefined
      264 |       awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
      265 |
    > 266 |       expect($thumbsUpEmoji.attr('title')).toBe('sam');
          |                                            ^
      267 |     });
      268 |   });
      269 |
      at Object.toBe (spec/frontend/awards_handler_spec.js:266:44)

How to set up and validate locally

I used the files from the master-broken incident as a use-case:

cat > changed_files.in <<FILE
app/views/award_emoji/_awards_block.html.haml
ee/app/views/groups/epics/show.html.haml
FILE

./tooling/bin/html_to_js_mappings changed_files.in related-js.out

yarn install

# Don't use the `--findRelatedTests` feature
$ yarn jest --findRelatedTests "" --passWithNoTests
No tests found, exiting with code 0

$ yarn jest --findRelatedTests $(cat related-js.out) --passWithNoTests
# You should see spec/frontend/awards_handler_spec.js being run in the specs

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #386719 (closed)

Edited by David Dieulivol

Merge request reports