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

(GH-397) - Honour default symlink when additional symlinks delcared #431

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

jordanbreen28
Copy link
Contributor

@jordanbreen28 jordanbreen28 commented Feb 1, 2024

Summary

Before, declaring a symlink would overwrite the symlink to the current module.
This is not what we would want for testing, and we should be able to declare additional symlinks to the default one.
Now, when extra symlinks declared, we merge into the auto_symlink, and if not, operate as before.

Additional Context

A .fixtures.yml like below will now work:

---
fixtures:
  repositories:
    puppetlabs-firewall:
      repo: https://github.com/puppetlabs/puppetlabs-firewall
      scm: git
    puppetlabs-peadm:
      repo: https://github.com/puppetlabs/puppetlabs-peadm
      scm: git
  symlinks:
      test_module: "#{source_dir}"
      peadm_spec: "#{source_dir}/spec/fixtures/modules/puppetlabs-peadm/spec/acceptance/peadm_spec/"

Console output:

I, [2024-02-01T17:04:02.236471 #84485]  INFO -- : Creating symlink from spec/fixtures/modules/test_module to /Users/jordan.breen/Desktop/repos/test_module
I, [2024-02-01T17:04:02.236812 #84485]  INFO -- : Creating symlink from spec/fixtures/modules/peadm_spec to /Users/jordan.breen/Desktop/repos/test_module/spec/fixtures/modules/puppetlabs-peadm/spec/acceptance/peadm_spec/

Related Issues (if any)

Fixes #397

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified.

Before, declaring a symlink would overwrite the symlink to the current
module.
This is not what we would want for testing, and we should be able to
declare additional symlinks to the default one.
Now, we extra symlinks declared, we merge into the auto_symlink, if not
operate as before.
@jordanbreen28 jordanbreen28 marked this pull request as ready for review February 1, 2024 17:06
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (157ed92) 41.71% compared to head (5745b70) 41.71%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #431   +/-   ##
=======================================
  Coverage   41.71%   41.71%           
=======================================
  Files          10       10           
  Lines         676      676           
=======================================
  Hits          282      282           
  Misses        394      394           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@LukasAud LukasAud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LukasAud LukasAud merged commit b2be9cc into main Feb 1, 2024
8 checks passed
@LukasAud LukasAud deleted the gh-397-honour_default_symlink branch February 1, 2024 17:21
@TheMeier
Copy link

TheMeier commented Feb 9, 2024

Well this breaks our ancient setup where we used puppetlabs_spec_helper for a control_repository. (yes I know there is better ways like onceover now)
Linking the control repo back into spec/fixtures/modules leads to this error:

     Puppet::ParseError:
       Internal Error: Attempt to redefine loader named '<path of the repo>'

@jordanbreen28
Copy link
Contributor Author

jordanbreen28 commented Feb 9, 2024

@TheMeier puppetlabs_spec_helper has the following that can be used for control repos

fixtures:
  repositories:
    firewall: "https://github.com/puppetlabs/puppetlabs-firewall.git"
    stdlib: "https://github.com/puppetlabs/puppetlabs-stdlib.git"
    control_repo:
      repo: "https://github.com/puppetlabs/control-repo"
      target: "spec/fixtures/control_repos"

you should be able to alter the target as you see fit. Hope this helps

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.

declaring a symlink should not alter the default symlink to current module
3 participants