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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Add support for 'using' keyword to dispose mocks/spies #14294

Closed
tzachbon opened this issue Jul 2, 2023 · 13 comments 路 Fixed by #14895
Closed

[Feature]: Add support for 'using' keyword to dispose mocks/spies #14294

tzachbon opened this issue Jul 2, 2023 · 13 comments 路 Fixed by #14895

Comments

@tzachbon
Copy link

tzachbon commented Jul 2, 2023

馃殌 Feature Proposal

Add support for the 'using' keyword, recently introduced in TypeScript 5.2, to handle the disposal of mock resources in Jest. This keyword provides a simple and efficient way to manage resources, and it could greatly enhance the cleanup process of mocks after each test run.

Motivation

The 'using' keyword in TypeScript 5.2 is designed to automatically dispose of anything with a Symbol.dispose function when it leaves scope. Given its utility in managing resources such as file handles and database connections, applying this feature to Jest's mock resources would provide a cleaner and more efficient way to ensure that mocks are properly cleaned up after each test run. This could reduce the risk of memory leaks or other unexpected behavior, enhancing the reliability and performance of tests.

Example

These are just some examples that I would imagine I will use if it will be natively supported from the returned value of "spyOn" or "mock"

describe('...', () => {
	it('...', () => {
		using spy = jest.spyOn(axios, 'get')
	})
})
{

  jest.mock('axios')

	using axios = require('axios')


	describe('...', () => {
		it('...', () => {
			// ...
		})
	})
}
{
	using mocked = jest.mock('axios')

	describe('...', () => {
		it('...', () => {
			// ...
		})
	})
}

Pitch

This feature aligns with the latest practices in TypeScript (and soon JavaScript), providing a more modern and efficient way to manage and dispose of mock resources. Given that Jest is a core tool for testing in the JavaScript ecosystem, it's crucial that it continues to evolve and incorporate these advancements. This feature would not just be a nice-to-have, but rather a significant improvement to Jest's handling of mock resources, making it a valuable addition to the core platform.

@tzachbon tzachbon changed the title [Feature]: Add support for 'using' keyword to dispose of mocks/spioes [Feature]: Add support for 'using' keyword to dispose of mocks/spies Jul 2, 2023
@mrazauskas
Copy link
Contributor

Could you elaborate on what you would expect from Symbol.dispose property in each of these cases? What should happen at the end of a block?

@SimenB
Copy link
Member

SimenB commented Jul 5, 2023

Something like this is super exciting, but I also don't know if we can really attach semantics here without breaking somebody's workflow

@tzachbon tzachbon changed the title [Feature]: Add support for 'using' keyword to dispose of mocks/spies [Feature]: Add support for 'using' keyword to dispose mocks/spies Jul 6, 2023
@tzachbon
Copy link
Author

tzachbon commented Jul 6, 2023

Could you elaborate on what you would expect from Symbol.dispose property in each of these cases? What should happen at the end of a block?

I would expect a full clean-up since the variable will be available only for the block:
1#

spyOn(...) {
	// ...
	[Symbol.dispose]() {
	  this.mockReset()
	}
}

I assume it is also relevant for jest.fn()

2#, 3#
Restore the request to the original module.

Something like this is super exciting, but I also don't know if we can really attach semantics here without breaking somebody's workflow

Why would it break? By using using you are expecting a dispose.
It won't change the behavior of var, const or let.

@mrazauskas
Copy link
Contributor

Thanks. I think it is worth to think if Symbol.dispose could be added to Mock Function which is the return value of both jest.spyOn() and jest.fn().

But I can鈥檛 see it working with the Jest Object which is the return value of jest.mock(). For instance, fake timers are part of the object. Should they be reset too? That鈥檚 tricky, because faking time is global operation. If Symbol.dispose on the Jest object resets the state of mocks and fake timers, very soon this can get messy.

(I talk about fake timers, because node:test added Symbol.dispose support for their timers recently. Reference nodejs/node#48549.)


In the using axios = require('axios') example the returned object could have Symbol.dispose method already. So if the loaded object will have Symbol.dispose method, it will work without any involvement from Jest side. Would be good not to break that and that鈥檚 it.


Probably it could work with jest.spyOn():

{
  using spiedMethod = jest.spyOn(someObj, 'someMethod')

  // do something

  // calls `spiedMethod.mockRestore()`
}

Not sure if that makes sense with jest.fn(). One can use const and will be simply garbage collected. Or did I miss something? Can鈥檛 see why there would be a need to restore it explicitly (well.. to be precise, Jest is restoring all mock functions after running each test file already).

@github-actions
Copy link

github-actions bot commented Aug 5, 2023

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Aug 5, 2023
@tzachbon
Copy link
Author

tzachbon commented Aug 8, 2023

@mrazauskas @SimenB - Just making sure, is this something you plan to adopt? Should I consider contributing to this :)

@mrazauskas
Copy link
Contributor

mrazauskas commented Aug 8, 2023

is this something you plan to adopt?

What is this in the sentence above?

In general it sounds useful. At this moment the concrete use cases are somewhat undefined. I understand it isn't easy to reason about the limitations without digging into the internals of Jest. If you feel like getting your hands dirty, just try putting together a PR.

@github-actions github-actions bot removed the Stale label Aug 8, 2023
@github-actions
Copy link

github-actions bot commented Sep 7, 2023

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Sep 7, 2023
@SimenB
Copy link
Member

SimenB commented Sep 7, 2023

PR welcome, yeah 馃憤

@SimenB SimenB added Pinned and removed Stale labels Sep 7, 2023
@SimenB
Copy link
Member

SimenB commented Sep 8, 2023

Example in the wild: apollographql/apollo-client#11177

@SimenB
Copy link
Member

SimenB commented Dec 28, 2023

@tzachbon are you still up for sending a PR here? 馃檪

@SimenB
Copy link
Member

SimenB commented Feb 20, 2024

https://github.com/jestjs/jest/releases/tag/v30.0.0-alpha.3

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants