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

Improve storing logs for the test suite run #605

Open
2 tasks done
danilchican opened this issue Feb 10, 2021 · 15 comments
Open
2 tasks done

Improve storing logs for the test suite run #605

danilchican opened this issue Feb 10, 2021 · 15 comments
Labels
enhancement Indicates new feature requests

Comments

@danilchican
Copy link

Checklist

Before making a feature request, I have:

Feature description

Currently pact.log is replaced every time pact test has ran.

Let's imagine that you already have pact tests and add one more. In case if one of the tests will fail, pact logs file can do not store logs for the failed test. It stores logs just for the latest test which was run.

I think it could be better if the logs file will contain all logs within a run test suite instead of replacing each time after specific test running.

Use case

In our project I faced with the described issue when I tried to add one more pact test and after running test suite I see that some tests are failed and I want to debug what happened. But when I opened logs file I saw that it stores only latest test run logs (not all of them). It's hard to debug. You need to rename or remove all pact tests except of failed and run test suite again but even in that case if you have more than one failing tests - in logs file will be logs for the only one.

@mefellows
Copy link
Member

Most test frameworks have a "focus" or "skip" type feature (i.e. in mocha you can add the suffix .only) that lets you run the single test that failed. So it doesn't seem like a huge burden to me, but the idea of having a log file with all of the interactions for a test run could makes sense.

How would you like this to function? Should the log file ever be truncated or should it always be appended to, and the user should be responsible for deleting it?

@danilchican
Copy link
Author

danilchican commented Feb 10, 2021

I think it would be good to have such flag in pact configuration like logsWriteMode with possible values overwrite, append/update and do not have possibility to store logs only for one test from test suite. It will be replaced with the following features like overwriting whole logs file or appending logs to existing one.
Overwrite mode will clean up the logs file before writing.

@TimothyJones
Copy link
Contributor

Jest pact helps this by giving each log file a different name, but you’re right-this could definitely be improved for the multi-suite use case.

I think we could work out a better-by-default method that would avoid the need to introduce additional config options

@TimothyJones
Copy link
Contributor

If you’re not using jest, then a workaround could be to instantiate one pact instance per suite- with different log files. (This is what jest-pact does)

@danilchican
Copy link
Author

@TimothyJones , yep. Even if default behavior will be improved to store all logs per suite like described above with overwrite option it's will be enough.

@TimothyJones
Copy link
Contributor

@danilchican what test runner are you using?

@danilchican
Copy link
Author

@TimothyJones, jest

@TimothyJones
Copy link
Contributor

Oh, right. Please check out https://github.com/pact-foundation/jest-pact - it solves this problem for you.

@danilchican
Copy link
Author

@TimothyJones , are you talking about -> https://github.com/pact-foundation/jest-pact#defaults ?

@TimothyJones
Copy link
Contributor

Apologies, looks like it's not in the documentation. I will correct that. If you start each mockserver on a different port and don't specify a log file, then the log file is named accordingly.

@danilchican
Copy link
Author

@TimothyJones , did you add this already in the documentation?

@TimothyJones
Copy link
Contributor

@TimothyJones
Copy link
Contributor

Sorry, preemptive send. I can’t seem to do a direct link on mobile. It’s in the defaults section of: https://github.com/pact-foundation/jest-pact

@danilchican
Copy link
Author

@TimothyJones
In previous version of my pact test I had a separate file with pact config of consumer, provider, mock url, etc.
But using this pact-jest package just for logs I have to use withPact method to wrap my consumer tests and put into method object with config.
Is there any possibility to load config from separate file for withPact as it was done before through --setupFiles ./config/pact/setup.js? I don't want to pass and duplicate config properties every time in every consumer test.

@TimothyJones
Copy link
Contributor

Ah, right. I see the problem.

The --setupFiles approach has a couple of drawbacks. It means the Pact mock is spun up for all specs, which adds time and noise to your tests. Additionally, it doesn't work especially well in cases where you need different settings for each spec (as you're currently experiencing). These are the problems that jest-pact was introduced to solve - but there are other workarounds (for example, splitting your specs into pact and non-pact). And, you raise a good point about not wanting to refactor your existing tests away from the --setupFiles method.

Using jest-pact with common config

The intention for multiple specs with common configuration in jest-pact is to have a common config object exported by a module:

import { withPact } from `jest-pact`;
import { pactConfig } from './__fixtures__/yourConfig';

withPact({...pactConfig, port: 8082 }, () => { /* etc */ });

However, long term we'd like to introduce / expose the config in a first-class way - see here for a proposal.

Solutions without migrating away from setupFiles

A naive way would be to add a random string to the log filename, but it would be annoying to tie those to the specs, and you would have to ensure you clear out the log directory before you run the tests.

I'll have a think about whether there's a way to write a --setupFiles module that would have sensible and repeatable log filenames for different tests. I don't think Jest exposes which spec is running. Hmm.

As Matt said above, you can also work around this by running just the spec you're interested in - although we realise that's not ideal.

Append log mode

As you said above, introducing an append log mode would also solve the problem in your case. Like the previous approach, a drawback would be that you'd have to ensure that the log files are cleaned out before running the tests. I'd really like the logs to work out of the box without extra config, though- making it configurable feels like we're leaking complexity to the user that would be better handled with sensible defaults (also- for technical reasons, adding this mode happens to not be a small change - but that doesn't mean we shouldn't do it, of course).

Obviating the need for the logs

As an aside- soon we'll have better exposition of the diff during the output of a failed test, which will mean needing to read the log file will happen less frequently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
Status: New Issue
Development

No branches or pull requests

3 participants