Skip to content

Delete vcr cassette for test case if it errors#6

Open
anujkumar93 wants to merge 2 commits intoBIOINF-982--mutation_apifrom
BIOINF-1146--del_vcr_on_error
Open

Delete vcr cassette for test case if it errors#6
anujkumar93 wants to merge 2 commits intoBIOINF-982--mutation_apifrom
BIOINF-1146--del_vcr_on_error

Conversation

@anujkumar93
Copy link
Copy Markdown
Owner

In production environments, it isn't trivial to delete the vcr cassettes before retriggering the build, when any exception in the test case occurs.

We want to delete the recorded cassette when an exception is raised as the last addCleanUp step, so that the module can be rebuilt without opening separate PRs about it.

@anujkumar93 anujkumar93 self-assigned this Oct 27, 2018
@jtratner
Copy link
Copy Markdown

I'm not sure this is a good idea. The cassettes should be generated on your local machine and the tests should pass on your local machine. This feels likely to make network requests if an exception occurs (and instead you should be manually deleting cassettes if a specific test is broken or wiping all of them and regenerating them as necessary)

@anujkumar93
Copy link
Copy Markdown
Owner Author

Actually, this does not make any network requests if an exception occurs in the test case. Plus as long as the cassettes are present, this won't come into play. If a test case breaks, this will delete the cassette sure. In the next build, it'll hit the network request, but isn't it better to just update an environment variable once during a build, rather than opening a PR to regenerate all cassettes?

@jtratner
Copy link
Copy Markdown

I understand the motivation; however, you can always see the cassette from the error message. I'd prefer not to do this, I don't think it adds value / has potential for network requests. Please don't merge this.

@anujkumar93
Copy link
Copy Markdown
Owner Author

Okay.

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.

2 participants