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

(PDK-381) Ensure spec fixtures are cleaned up, even if the test fails #204

Merged
merged 1 commit into from
Aug 10, 2017

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented Aug 10, 2017

Currently if the rspec tests fail, the spec_clean task is not invoked as rake aborts. By moving the spec_clean task into an ensure block, it will get run and clean things up even when the rspec tests fail.

@puppetcla
Copy link

CLA signed by all contributors.

@DavidS DavidS merged commit 920d647 into puppetlabs:master Aug 10, 2017
@rodjek rodjek deleted the pdk-381 branch August 11, 2017 01:03
@ekohl
Copy link
Contributor

ekohl commented Sep 7, 2017

I've noticed this and relied on this behaviour. It bit me a few times, for example when I was developing in the airport on slow wifi and accidentally ran (out of custom) rake spec rather than rspec on the failing test.

Is it possible to revert this or make it configurable?

@DavidS
Copy link
Contributor

DavidS commented Sep 8, 2017

The previous behaviour can be achieved by running rake spec_prep spec_standalone. Admittedly, that won't help you, if you inadvertently run rake spec, or do anything else that poisons your fixtures.

@ekohl
Copy link
Contributor

ekohl commented Sep 8, 2017

@DavidS you're right that I can achieve it, but as a user I was surprised that in a patch release this changed.

@ekohl
Copy link
Contributor

ekohl commented Sep 8, 2017

Also I realise this is a bit of https://xkcd.com/1172/

@DavidS
Copy link
Contributor

DavidS commented Sep 11, 2017

Thanks for your understanding, and apologies for breaking your workflow.

@sean797
Copy link
Contributor

sean797 commented Sep 12, 2017

ugh, this broke some of my workflows too. #211 should fix mine, not sure if it will also fix your problem @ekohl

@ekohl
Copy link
Contributor

ekohl commented Sep 15, 2017

Another side-effect is that when you use parallel_spec and cancel tests (ctrl+c) that it starts cleaning up fixtures while some tests are still running. This leads to unexpected failures like:

       Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, Could not find class ::foreman::params for donald.ens.kohlvanwijngaarden.nl at /home/ekohl/dev/puppet-foreman_proxy/spec/fixtures/modules/foreman/manifests/repos.pp:6:3 at /home/ekohl/dev/puppet-foreman_proxy/spec/fixtures/modules/foreman_proxy/manifests/install.pp:4 on HOSTNAME
       Could not find parent resource type '::dhcp::params' of type hostclass in rp_env at /home/ekohl/dev/puppet-foreman_proxy/spec/fixtures/modules/dhcp/manifests/init.pp:1 on node HOSTNAME
       Evaluation Error: Error while evaluating a Function Call, Could not autoload puppet/parser/functions/cache_data: No such file or directory @ realpath_rec - /home/ekohl/dev/puppet-foreman_proxy/spec/fixtures/modules/extlib/lib/puppet/parser/functions/cache_data.rb at /home/ekohl/dev/puppet-foreman_proxy/spec/fixtures/modules/foreman_proxy/manifests/params.pp:349:28 on node HOSTNAME

You don't see these when you let the tests continue to run. This is a system with 8 CPUs.

@DavidS
Copy link
Contributor

DavidS commented Sep 18, 2017

@ekohl that sounds like an issue with the parallel_tests library rather than our usage.

@ekohl
Copy link
Contributor

ekohl commented Sep 18, 2017

@DavidS possibly, but in the end I'm using the spec helper and this is the experience it provides me.

@DavidS
Copy link
Contributor

DavidS commented Sep 18, 2017

grosser/parallel_tests#523

@ekohl
Copy link
Contributor

ekohl commented Sep 18, 2017

Thanks, that does explain it.

@chelnak chelnak added the bug label Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants