Draft: Fail feature specs on unexpected browser console errors - no allow list (DON'T MERGE)

Miguel Rinconrequested to merge
feature-specs-fail-on-browser-console-errors into master

What does this MR do and why?

This is a reveal MR for !256639. It is not meant to merge as is. Its purpose is to run the full feature spec suite with a stricter browser console check and list the errors that Vue 2 hides today.

This MR changes the feature spec browser console check to fail on any SEVERE console error, not only when the example already failed. Vue 2 catches render errors, logs them with console.error, and renders the component blank. Vue 3, in the development build that feature specs use, rethrows these errors and breaks the component tree. Feature specs pass under Vue 2 today while logging these latent bugs. With this change, the specs fail now, before a Vue 3 migration flag is enabled. This applies to every page, not only Rapid Diffs.

Changes:

  • spec/support/capybara.rb, config.after(:example, :js) hook: raise_if_unexpected_browser_console_output runs for every :js example, except when the exception is RSpec::Core::Pending::PendingExampleFixedError. Before, it ran only when the example had already failed.
  • spec/support/helpers/browser_console_helpers.rb, BROWSER_CONSOLE_ERROR_FILTER: also ignores Failed to load resource (Chrome logs every 4xx/5xx response as SEVERE; many specs assert on them), all Snowplow tracker messages (CSP violations in the test environment), and [Sentry stub] (the stub forwards captureException to console.error). The first pipeline run showed that these three messages made up most of the failures.

Note that [Vue warn] messages are also emitted with console.error and therefore are SEVERE. They also fail specs now.

The pipeline runs all RSpec jobs and the Vue 3 system jobs. Expect many failures. Each failure is either a latent bug that a Vue 3 migration would turn into a crash, noise that the filter should ignore, or a spec that must be fixed.

Findings from the first run

Pipeline https://gitlab.com/gitlab-org/gitlab/-/pipelines/2843011566 (before the filter widening). The expected Rapid Diffs failure was present under Vue 2 ([Vue warn]: Error in render: "TypeError: Cannot read properties of undefined (reading 'id')" in user_comments_on_whitespace_hidden_diff_spec.rb and user_sees_avatar_on_diff_notes_spec.rb). Other application errors found, in both Vue versions unless noted:

  • Uncaught ReferenceError: _this12 is not defined (main.chunk.js): analytics dashboards, compliance dashboard, value streams dashboard
  • Uncaught ParametersMissing: Route missing required keys: id: Pages domains and Let's Encrypt settings
  • Uncaught RangeError: Invalid time value: branches page
  • Uncaught TypeError: Cannot read properties of null (reading 'text'): note comments, GFM autocomplete, design comments
  • Uncaught TypeError: Cannot read properties of null (reading 'containsElement'): incident timeline events, work item statuses, service account tokens
  • Uncaught Error: No match for (Vue 3 jobs only): merge request list filtering and sorting
  • Cannot set properties of null (setting '__vnode') (Vue 3 jobs only): Rapid Diffs reply placeholder, and Merge request > Batch comments ... resolves the thread
  • Smaller groups: GlTabsBehavior without an element, reading 'appendChild', reading 'value', reading 'filter', reading 'map', reading 'emitsOptions', $toolpop.show is not a function, reading 'clientWidth', several [Vue warn]: Invalid prop, Apollo Missing field 'humanTimeEstimate'

References

Screenshots or screen recordings

This MR has no UI change. Screenshots are not required.

How to set up and validate locally

Run a feature spec that has a known console error on master. For example:

bin/rspec spec/features/merge_request/user_comments_on_whitespace_hidden_diff_spec.rb:49

This spec targets Rapid Diffs. Under Vue 2 it passes on master and logs TypeError: Cannot read properties of undefined (reading 'id'). With this MR, it fails with BrowserConsoleError and prints the console output. With the fix from !255075 (merged) applied, it passes again.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

Edited by Miguel Rincon

Merge request reports