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

test(logger): migrate to vitest & custom matchers #3131

Merged
merged 4 commits into from
Sep 27, 2024
Merged

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Sep 27, 2024

Summary

Changes

Please provide a summary of what's being changed

This PR migrates the unit tests for the Logger utility to use Vitest rather than Jest.

With this change, we can also finally start investing time/effort in custom matchers that can further reduce the boilerplate we write in our tests.

The first example of this are two new custom matchers introduced in this PR:

process.env.POWERTOOLS_DEV = 'true';
vi.spyOn(console, 'info').mockReturnValue();

// Matches any of the logs emitted
expect(console.info).toHaveLogged(
  expect.objectContaining({
    message: 'Hello, world!',
  })
);

// Matches the log emitted at the specified nth call
expect(console.info).toHaveLoggedNth(
  1,
  expect.objectContaining({
    message: 'Hello, world!',
  })
);

These two matchers will be available across all the test suites of our project and allow you to assert that a mocked console method has emitted a specific JSON-structured object.

Before this change, to use the matcher we had to directly drill down the mock calls and manually parse the object each time like this: expect(JSON.parse(logSpy.mock.calls[0][0])).toEqual(...).

These new matchers, in addition to providing a more ergonomic call signature, also provide better error messaging when tests fail. The new matchers leverage framework APIs and provide a more targeted diff between expected and received objects.

Besides the above, the tests are largely unchanged except for 2 new test cases being added due to the new coverage reporter (now using v8 instead of istanbul).

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: closes #3130


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this Sep 27, 2024
@dreamorosi dreamorosi requested review from a team as code owners September 27, 2024 13:49
@boring-cyborg boring-cyborg bot added automation This item relates to automation dependencies Changes that touch dependencies, e.g. Dependabot, etc. internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) logger This item relates to the Logger Utility tests PRs that add or change tests labels Sep 27, 2024
@pull-request-size pull-request-size bot added the size/XL PRs between 500-999 LOC, often PRs that grown with feedback label Sep 27, 2024
Copy link

@am29d am29d merged commit 6b65ab4 into main Sep 27, 2024
22 checks passed
@am29d am29d deleted the test/logger_vitest branch September 27, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation This item relates to automation dependencies Changes that touch dependencies, e.g. Dependabot, etc. internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) logger This item relates to the Logger Utility size/XL PRs between 500-999 LOC, often PRs that grown with feedback tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: migrate test runner to vitest for logger package
2 participants