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

Clear out pact directory during setup? #102

Open
TimothyJones opened this issue Sep 11, 2019 · 11 comments
Open

Clear out pact directory during setup? #102

TimothyJones opened this issue Sep 11, 2019 · 11 comments

Comments

@TimothyJones
Copy link
Contributor

Usually I end up wrapping the test script with something that clears out the pacts folder - it would be great if we could do this automatically.

@YOU54F
Copy link
Member

YOU54F commented Oct 5, 2019

that sounds like a good idea, we could use something like rimraf. (https://github.com/isaacs/rimraf)

do you store each pact in a pacts folder, as a root file, or in subfolders? i store consumer/provider pairs in pacts folder, non nested

If we delete the entire folder, we would need to consider the use of multiple test scripts running in parallel, in a single repo, as we couldn't delete the whole folder, as we may lose pacts generated by other tests.

@YOU54F
Copy link
Member

YOU54F commented Apr 25, 2022

Thinking about this, as we know the users pact directory, we could add a flag to say clearPactDirBeforeTest?

@TimothyJones
Copy link
Contributor Author

You have to do this at test setup time, which is a different time to the time all the current jest-pact code runs (because you can have pactWith in multiple spec files).

You can do it with a global setup module exported by jest-pact - see here https://jestjs.io/docs/configuration#globalsetup-string . We could do jest-pact/clearPactFolder or similar.

However, to implement it, you need to know what the test directory is. The setup is currently passed to pactWith, which would not be readable during the setup time.

The plan I was loosely thinking was to implement pactrc config, which would obviate the need for the "sensible defaults" used in jest-pact (because we could implement / move that implementation there instead).

@YOU54F
Copy link
Member

YOU54F commented Apr 25, 2022

Hmmm, all very good points. Been a while since I've dived into Jest's innard, but there were some interesting things that came out of the pact-watch discussion that might allow us more configurability

I love the idea of a pact config file, and inspired by some of the experiences of CLI tools, so have been experimenting with some projects to scaffold a repo or setup pact in a project.

I think there is so much pain felt from things like env vars (different shells/os's/Makefiles), that reading sensible defaults from a config file, would be a nice feature.

Wonder if we might want a generic json one, and then language specific ones, as I'm sure each language has its own convention for setup files?

@TimothyJones
Copy link
Contributor Author

Project-wide, we might want a general pact config. I was thinking we'd implement something that works for pact-js, and then see if other projects wanted to pick it up (although I was also thinking that anything language specific would go in its own key in the JSON).

@TimothyJones
Copy link
Contributor Author

I don't know why I'm saying we. I'm not planning to do any of this work now that I'm no longer a maintainer. Sorry.

@YOU54F
Copy link
Member

YOU54F commented Apr 25, 2022

It's okay @TimothyJones I know it is the collective we ❤️

Thanks for all your many contributions, and still caring about beautiful developer experiences.

@radiovisual
Copy link

What about not clearing the pact directory on setup, but instead write the pact to temporary files, only updating the official pact files when/if the test run is successful? Seems pretty destructive to nuke the pact directory even if the tests fail...or am I missing something?

@mefellows
Copy link
Member

That just makes the test suite more complicated, because you need to tell pact the merge / replace the file when it's done. Technically the tests can be run in parallel which is why it's necessary to remove it in advance, to ensure no stale/outdated interactions remain there.

There's nothing important about the files so nuking them in advance shouldn't be a problem.

It's probably only a problem if you publish from your dev machine (which is not recommended) because you're unlikely to have stale interactions on CI

@radiovisual
Copy link

Yes, true. It would complicate things with little benefit...

@TimothyJones
Copy link
Contributor Author

It's probably only a problem if you publish from your dev machine (which is not recommended)

It's also a problem if you commit your pact files (which is also not recommended, although I don't recall it being explicit in the documentation). I've definitely seen installs where this is the case.

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

No branches or pull requests

4 participants