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

.fixtures.yml: Migrate to git #151

Merged
merged 11 commits into from Aug 22, 2023
Merged

.fixtures.yml: Migrate to git #151

merged 11 commits into from Aug 22, 2023

Conversation

bastelfreak
Copy link
Contributor

@bastelfreak bastelfreak commented Aug 22, 2023

it's best practice to use git branches in unit tests, not the latest forge releases. This approach tries to catch breaking changes or regressions in depenencies before they are released.

also contains:

@bastelfreak bastelfreak force-pushed the fixtures branch 3 times, most recently from a3ec3ab to f1b71f0 Compare August 22, 2023 09:18
This is a requirement to update the module dependencies. The
dependencies don't support such ancient operating systems.
stdlib 9 namespaced all functions.
This is required for puppetlabs/stdlib 9.
it's best practice to use git branches in unit tests, not the latest
forge releases. This approach tries to catch breaking changes or
regressions in depenencies before they are released.
@bastelfreak
Copy link
Contributor Author

@ju5t this is the final PR. I tried to split up all changes in individual PRs so we get a proper changelog.

This was referenced Aug 22, 2023
@ju5t
Copy link
Collaborator

ju5t commented Aug 22, 2023

I don't find this approach very developer friendly. If I make a PR to a module, I would like to start with green tests. With this change, it's not unlikely that our tests will fail because a third party module has introduced a breaking change or changed requirements.

It could potentially lead to delayed releases too. We/contributors would have to implement fixes for all the breaking changes.

Personally, I wouldn't like that.

@bastelfreak
Copy link
Contributor Author

if you don't like it you can also merge #155, which is the previous PR. I want to point out that the pinned versions in .fixtures.yml are several years old. At the moment the module doesn't work with the versions it mentions in the metadata.json. It provides a false impression. IMO that's worse compared to testing against main/master branches. You need to implement fixes for breaking changes anyway. The question is do you catch them before doing a release or afterwards. Because of that, we (Vox Pupuli) and Puppet switched to testing against branches years ago. That's our recommended best practice. For acceptance tests with beaker/litmus, we test against the actual forge releases.

@ju5t
Copy link
Collaborator

ju5t commented Aug 22, 2023

I agree, testing against old versions makes no sense.

Would it be possible to pick the latest released version somehow, instead of the main branch? There can be many changes before a new version gets released and that's not always a good thing.

@bastelfreak
Copy link
Contributor Author

as far as I know puppetlabs_spec_helper doesn't support that at the moment :(
If you like I can also lookup the latest releases and pin to them for now.

@ju5t
Copy link
Collaborator

ju5t commented Aug 22, 2023

We'll go the YOLO path for now. Thanks for the insane amount of work!

@ju5t ju5t merged commit 457b065 into sensson:main Aug 22, 2023
4 checks passed
@bastelfreak bastelfreak deleted the fixtures branch August 22, 2023 12:51
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.

None yet

2 participants