-
Notifications
You must be signed in to change notification settings - Fork 783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
html-reporter: urlParams checkboxes broken (hidepassed, noglobals, notrycatch) #1657
Comments
Follows-up cec22c1, which changed the HTML Reporter to consume data from the "runStart" event. This event is fired immediately before "begin" callback, where an earlier listener from urlparams.js is responsible for applying the URL parameters to the QUnit.config object. Fixes qunitjs#1657.
It already confirmed that it worked end-to-end from URL parameter to visible behaviour. But, we did not confirm the intermediary effect (exposed as parsed config property), and the side-effect of the interface reflecting the correct state. This is a regression test for bug qunitjs#1657.
Similar to 87c1f9f, but for the `filter` config. * Move general behaviour test to a CLI fixture. * Expand coverage of the HTML file by starting from a real URL parameter instead of setting the config option at run-time. * Expand coverage by verifying that the option is parsed into the config object. * Expand coverage by verifying that the HTML Reporter displays the value in the input field. Follows-up qunitjs#1657.
Thanks @bastimeyer. It was embarrassing to see this slip through our tests. We have lots of tests in place that verify that What we lacked was an end-to-end test for the three most important config options (those in the UI), and to simply check that the UI displays their current values correctly.
I understand the use case and I'd like to explore this in a separate issue. My concern here is backward-compatibility as this is not something we have supported in the past and is potentially incompatible with some of the callback features for plugins and configuration. My understanding is that most use of |
It already confirmed that it worked end-to-end from URL parameter to visible behaviour. But, we did not confirm the intermediary effect (exposed as parsed config property), and the side-effect of the interface reflecting the correct state. This is a regression test for bug qunitjs#1657.
Similar to 87c1f9f, but for the `filter` config. * Move general behaviour test to a CLI fixture. * Expand coverage of the HTML file by starting from a real URL parameter instead of setting the config option at run-time. * Expand coverage by verifying that the option is parsed into the config object. * Expand coverage by verifying that the HTML Reporter displays the value in the input field. Follows-up qunitjs#1657.
It already confirmed that it worked end-to-end from URL parameter to visible behaviour. But, we did not confirm the intermediary effect (exposed as parsed config property), and the side-effect of the interface reflecting the correct state. This is a regression test for bug #1657.
Similar to 87c1f9f, but for the `filter` config. * Move general behaviour test to a CLI fixture. * Expand coverage of the HTML file by starting from a real URL parameter instead of setting the config option at run-time. * Expand coverage by verifying that the option is parsed into the config object. * Expand coverage by verifying that the HTML Reporter displays the value in the input field. Follows-up #1657.
Tell us about your runtime:
2.17.1
NW.js - both: Chromium 93 + Node 16
manually in NW.js (dev) / custom QUnit reporter/bridge via Chrome Debugging Protocol (CLI / headless)
with autostart=false and delayed/injected QUnit.start() call
Issue
The
hidepassed
,noglobals
andnotrycatch
checkboxes are completely broken. Neither the checked state is set correctly, nor does the toggle work properly.This is especially frustrating when default config values are set.
Observation
QUnit seems to build the
urlParams
object right at the start when loading the module.https://github.com/qunitjs/qunit/blob/2.17.1/src/html-reporter/urlparams.js#L12-L14
However, when building the checkboxes in
getUrlConfigHtml
, the config values are not set yet from theurlParams
, so it's always setting the checkboxes' checked state from the default config values:https://github.com/qunitjs/qunit/blob/2.17.1/src/html-reporter/html.js#L156-L162
That's because the
begin
callback where the config values are set from theurlParams
is run afterwards and too late:https://github.com/qunitjs/qunit/blob/2.17.1/src/html-reporter/urlparams.js#L56-L72
Also, when deselecting a checkbox and there's no entry in the
window.location.search
query string, default config values are always applied instead of explicit false values representing the state of the unchecked checkboxes. Forhidepassed
, this is no problem because it doesn't refresh the page, but for the other urlParams entries it is.Expected behavior
QUnit should allow setting default config values and should correctly handle the checked state of the urlParams checkboxes, in addition to false values of urlParams entries for being able to toggle the state despite of the default values.
Thanks!
The text was updated successfully, but these errors were encountered: