Skip to content
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

Improve unit test usability #2463

Merged
merged 1 commit into from Mar 8, 2024
Merged

Conversation

kzar
Copy link
Collaborator

@kzar kzar commented Mar 7, 2024

The unit tests were a little tricky to work with, for failures the
wrong line numbers were given and console.log() didn't work. Let's try
to improve the situation:

  1. Have the unit tests bundled with inline sourcemaps, that can then
    be used to report the line numbers correctly.
  2. Add a note to the unit test's README to mention to use
    console.warn() or console.error() instead of console.log().

Note: Unfortunately adding inline sourcemaps seems to slow down
Karma/Jasmine quite a bit. In the future, we might consider
using a faster test runner, or one that supports external
sourcemaps.

Reviewer: @sammacbeth
CC: @jdorweiler

Description:

Steps to test this PR:

  1. Ensure the unit tests still pass.
  2. Add an error throw or failing assertion to one of the unit tests.
  3. Re run the unit tests and make sure the reported line numbers are correct.

Automated tests:

  • Unit tests
  • Integration tests
Reviewer Checklist:
  • Ensure the PR solves the problem
  • Review every line of code
  • Ensure the PR does no harm by testing the changes thoroughly
  • Get help if you're uncomfortable with any of the above!
  • Determine if there are any quick wins that improve the implementation
PR Author Checklist:
  • Get advice or leverage existing code
  • Agree on technical approach with reviewer (if the changes are nuanced)
  • Ensure that there is a testing strategy (and documented non-automated tests)
  • Ensure there is a documented monitoring strategy (if necessary)
  • Consider systems implications

The unit tests were a little tricky to work with, for failures the
wrong line numbers were given and console.log() didn't work. Let's try
to improve the situation:

1. Have the unit tests bundled with inline sourcemaps, that can then
   be used to report the line numbers correctly.
2. Add a note to the unit test's README to mention to use
   console.warn() or console.error() instead of console.log().

Note: Unfortunately adding inline sourcemaps seems to slow down
      Karma/Jasmine quite a bit. In the future, we might consider
      using a faster test runner, or one that supports external
      sourcemaps.
@kzar kzar requested a review from sammacbeth March 7, 2024 18:05
@kzar kzar merged commit 3b02ef5 into duckduckgo:main Mar 8, 2024
24 of 25 checks passed
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.

None yet

2 participants