Skip to content

Mock console logging during tests + use explicit resource management for spies#4138

Open
eemeli wants to merge 3 commits into
mozilla:mainfrom
eemeli:mock-better
Open

Mock console logging during tests + use explicit resource management for spies#4138
eemeli wants to merge 3 commits into
mozilla:mainfrom
eemeli:mock-better

Conversation

@eemeli
Copy link
Copy Markdown
Member

@eemeli eemeli commented May 7, 2026

A few of the tests that were ported to vitest are logging to the console during normal runs; let's mock them and test that they are called instead.

Also, with explicit resource management (using instead of const) the mocks get automatically cleaned up.

@eemeli eemeli requested a review from nishitmistry May 7, 2026 09:39
@mathjazz
Copy link
Copy Markdown
Collaborator

mathjazz commented May 7, 2026

Please fix the failing tests.

@eemeli
Copy link
Copy Markdown
Member Author

eemeli commented May 18, 2026

The fix here was to update the Node.js used in tests to v24, which is what we're already using in the Docker containers.

I'd misread the release notes on the introduction of explicit resource management; that didn't actually become available until Node.js v24.

@mathjazz
Copy link
Copy Markdown
Collaborator

Hi @nishitmistry, do you have cycles to review this?

@nishitmistry
Copy link
Copy Markdown
Collaborator

Hi @nishitmistry, do you have cycles to review this?

hi @mathjazz, yes

Comment thread package.json
]
},
"engines": {
"node": ">= 24",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you forget to push the updated package-lock.json file.

@nishitmistry
Copy link
Copy Markdown
Collaborator

rest lgtm

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.

3 participants