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

Fix/556 grab side effect #568

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

smoser
Copy link
Contributor

@smoser smoser commented Nov 30, 2023

  • fix: Remove the bin/ dir that was created as a side-effect of grab.

    'stacker grab' was creating a 'bin/' dir in the current working
    directory. This fixes that and adds a test to ensure that neither
    grab nor build have side effects.

    Fixes Bug: stacker grab has side-effects creates bin/ #566.

  • test: separate detection of overlay support from every stacker run.

    In unpriv test mode, we were running 'stacker testsuite-check-overlay'
    every time we ran stacker.

    That was causing me confusion and also is just wasteful, as the ability
    to use overlay does not change within the course of a test run.

    This introduces 'suite_setup' which ideally would be used by the bats
    'suite_setup' functionality. However, the Ubuntu 22.04 verison of bats
    does not have that functionalty.

    When run with '-j', this code will run more than once, but will worst
    case only run once per test, rather than once per invocation of
    'stacker' (some tests invoke stacker many times).

    Also, adjust output of run_stacker function to be more useful
    than just "Debug mode: "

@hallyn
Copy link
Contributor

hallyn commented Jan 8, 2024

Hm, the unpriv-setup caching does not appear to be working:

6384 2024-01-08T20:13:22.3302208Z UNPRIV_OVERLAY_SUPPORT was not set; stacker_setup must be called
6385 2024-01-08T20:13:22.3303039Z not ok 1 setup_file failed
6386 2024-01-08T20:13:22.3304230Z # (from function `source' in file test/helpers.bash, line 47,
6387 2024-01-08T20:13:22.3305441Z #  from function `load' in file /usr/lib/bats-core/test_functions.bash, line 29,
6388 2024-01-08T20:13:22.3307032Z #  from function `source' in file test/annotations-namespace.bats, line 1,
6389 2024-01-08T20:13:22.3308539Z #  from function `bats_run_setup_file' in file /usr/libexec/bats-core/bats-exec-file, line 81,
6390 2024-01-08T20:13:22.3310015Z #  from function `main' in test file /usr/libexec/bats-core/bats-exec-file, line 207)
6391 2024-01-08T20:13:22.3311038Z #   `bats_run_setup_file' failed

@smoser smoser force-pushed the fix/556-grab-side-effect branch 3 times, most recently from 445fb8f to a30061d Compare January 9, 2024 21:26
Copy link

codecov bot commented Jan 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2152d8d) 57.17% compared to head (f0f17c2) 57.16%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #568      +/-   ##
==========================================
- Coverage   57.17%   57.16%   -0.01%     
==========================================
  Files          64       64              
  Lines        7521     7520       -1     
==========================================
- Hits         4300     4299       -1     
  Misses       2479     2479              
  Partials      742      742              

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

'stacker grab' was creating a 'bin/' dir in the current working
directory.  This fixes that and adds a test to ensure that neither
grab nor build have side effects.

Fixes project-stacker#566.

Signed-off-by: Scott Moser <smoser@brickies.net>
@smoser
Copy link
Contributor Author

smoser commented Jan 12, 2024

Interesting, the failure here for test coverage is because there is 1 less line of code in a file, but the same coverage (both the removed and the added lines were tested).

$ git show pkg/stacker/grab.go | diffstat -p1
 pkg/stacker/grab.go |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

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

lgtm

@hallyn hallyn merged commit 583c4f6 into project-stacker:main Jan 12, 2024
10 of 11 checks passed
@hallyn
Copy link
Contributor

hallyn commented Jan 12, 2024

Thanks

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.

Bug: stacker grab has side-effects creates bin/
3 participants