avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Add ability to reverse the order of teardown steps #2495

Closed marchuffnagle closed 4 years ago

marchuffnagle commented 4 years ago

The new teardown functionality is awesome. In most of my tests, though, I need the teardown operations to happen in the opposite order that they were specified.

For example, let's say I have a test that goes something like this:

In this contrived example, let's pretend that you couldn't just recursively delete the directory. My test would do something like this:

The nice thing about declaring the teardown steps as you go is that, if something throws an exception, it will still clean up the resources that have been created to that point.

In this example, though, when ava runs the teardown steps, the first thing it's going to do is try to delete the directory, which is going to fail because the directory isn't empty.

It would be great if there was a say to say "run teardown steps in reverse".

Thanks!

novemberborn commented 4 years ago

In most of my tests, though, I need the teardown operations to happen in the opposite order that they were specified.

Yes you're right @yjpa7145. Not sure why we didn't do that.

Unfortunately we documented that the teardown functions run in order, so I don't think we can get away with changing this in a minor version.

The way to land this now is to make it an experimental feature. Let's call it reverseTeardowns. Add that to the set here:

https://github.com/avajs/ava/blob/baaf99a792eed586678a0cf88864f3f0aa16bd7a/lib/load-config.js#L10

Then to enable this experiment, add nonSemVerExperiments: { reverseTeardowns: true } to your AVA configuration.

Rather than pushing new functions we should unshift, but only if the experiment is enabled. You can access that through this.experiments.reverseTeardowns here:

https://github.com/avajs/ava/blob/baaf99a792eed586678a0cf88864f3f0aa16bd7a/lib/test.js#L481

And we should add this to the documentation, too.

I'll be sure to add an issue for the next major milestone to make this the default behavior.

marchuffnagle commented 4 years ago

Sounds good, I’ll pull the repo this week and try to take a crack at it.

devil-cyber commented 4 years ago

I am new to open source can I work on this

novemberborn commented 4 years ago

@devil-cyber let's give @yjpa7145 a chance to pick this up first 😄

marchuffnagle commented 4 years ago

Thanks, I'm on it. Should have a PR up today. I appreciate the enthusiasm, though, @devil-cyber.

marchuffnagle commented 4 years ago

@novemberborn I have a PR up, let me know if there's anything you'd like to see changed. I opted for reversing the list of teardowns rather than conditionally using push vs unshift so that it was a little more explicit why it was happening.

novemberborn commented 4 years ago

Done in https://github.com/avajs/ava/pull/2505.