Skip to content

Conversation

@cconard96
Copy link
Contributor

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.

Description

The current plain JS scripts are probably stable but adding these tests should still help, especially if they get redone in an upcoming version.
An additional perk is that it forces me to review the current functions in the scripts and I can identify which ones could be removed in the next major version rather than being migrated/refactored. For common.js there are at least 17% of functions that are unused or could be easily be replaced by better alternatives.

It isn't my intention to test everything, in fact it isn't even possible without complex mocks, but I've been caught up on the current test coverage and for Jest tests it has been sitting at around just 13% for a while now. I am not sure E2E tests cover a lot of it either. Even if they are changed to ESM/Vue code later, these tests are likely to still be useful.

Jest tests have been changed to output the code coverage now. Even though it isn't tracked in a central location, it would be nice to be able to see it at least in the CI runs.

The only code changes here are:

  • Change done to then on jQuery AJAX calls for compatibility with the AJAX mock system in the tests. Functionality should remain the same.
  • Change innerText to textContent as innerText is not implemented in JSDom. There are some differences, but none that should matter in the context of the system information.
  • Addition of CJS exports conditionally if module.exports is defined. In other words, some functions that are tested are exported using node's module system when in the test environment. This avoids the need to explicitly add the functions to the window. Should have no effect in the browser.

@cconard96 cconard96 marked this pull request as ready for review December 28, 2025 11:45
@cedric-anne cedric-anne added this to the 11.0.5 milestone Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants