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-636) Always remove symlink fixtures. Only remove downloaded fixtures if tests pass. #241

Merged
merged 1 commit into from
May 8, 2018

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented May 8, 2018

Restores most of the original behaviour of running spec, so that downloaded fixtures are only cleaned up if the tests pass. Fixtures created from symlinks (which cause problems when kept around) are still always removed at the end of each test run regardless of pass or fail.

@DavidS DavidS merged commit 0db95af into puppetlabs:master May 8, 2018
@rodjek rodjek deleted the pdk-636 branch May 8, 2018 09:57
@farkasmate
Copy link

For me it sounds like it's not fulfilling the second acceptance criteria:
on a module with fixtures, running pdk test unit a second time has much lower setup time

pdk test unit using PDK 1.4.1 takes about 10 seconds with an empty class and a generated test to finish.
If I'm adding puppetlabs-stdlib and puppetlabs-concat as dependencies the same run takes about 24 seconds to finish.

In my reading the ticket is about to make the second execution time close to the execution time of a test without dependencies - 10 seconds in the example above.

@DavidS
Copy link
Contributor

DavidS commented May 8, 2018

I've updated PDK-636 to reflect the current status of development of this improvement accurately.

@rodjek
Copy link
Contributor Author

rodjek commented May 8, 2018

@farkasmate are those modules being pulled in from git or the forge?

@farkasmate
Copy link

Forge.

---
fixtures:
  forge_modules:
    concat:
      repo: 'puppetlabs-concat'        
      ref: '4.2.0'
    stdlib:
      repo: 'puppetlabs-stdlib'        
      ref: '4.24.0'

@alex-harvey-z3q
Copy link
Contributor

This change is going to change the behaviour of the spec_clean task, which is going to confuse people I think, considering it's had the same behaviour for many years.

@farkasmate
Copy link

@alexharv074 I agree, it might get confusing.

I prefer the current behaviour in CI, but for local runs I'd like to keep the cached fixtures to speed up additional runs.

Ideally I'd trigger the new behaviour by specifying an option on the command line or setting a value in a config file under ~/.pdk/.

@rodjek
Copy link
Contributor Author

rodjek commented May 8, 2018

Yeah, I think it makes sense to see these as separate changes. This change restores the behaviour of the spec_clean task to what people are used to. We can add a flag to pdk test unit to not clean up the downloaded fixtures (even on a successful run) to handle the quicker local interative workflow side of things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants