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

core/cliwrap: Fix is_ostree_layout() in unit tests #5241

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

cgwalters
Copy link
Member

The is_ostree_layout() function parsed the real root filesystem. But we started using it in unit tests which creates unpredictable behavior.

The unit test for the core script wrapping was failing for me, and digging in I don't see how it could have ever passed. I don't think we're actually running the Rust tests in CI...need to dig into that.

  • Change is_ostree_layout to take a cap-std root
  • Add a unit test for it alone
  • Change all users to pass the rootfs
  • Fix the undo() method to undo the kernel-install wrapping correctly

With this the unit test passes again, and we shouldn't have any behavior which depends on the development root.

The `is_ostree_layout()` function parsed the real root filesystem.
But we started using it in *unit tests* which creates unpredictable
behavior.

The unit test for the core script wrapping was failing for me,
and digging in I don't see how it could have ever passed. I don't
think we're actually running the Rust tests in CI...need to dig
into that.

- Change `is_ostree_layout` to take a cap-std root
- Add a unit test for it alone
- Change all users to pass the rootfs
- Fix the `undo()` method to undo the kernel-install wrapping
  correctly

With this the unit test passes again, and we shouldn't
have any behavior which depends on the development root.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters cgwalters requested a review from jmarrero January 22, 2025 21:37
@cgwalters
Copy link
Member Author

/retest

@cgwalters
Copy link
Member Author

Quay flakes...

@cgwalters
Copy link
Member Author

/retest

@cgwalters cgwalters enabled auto-merge January 23, 2025 12:50
Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@cgwalters cgwalters merged commit e64d9da into coreos:main Jan 23, 2025
17 checks passed
This was referenced Jan 23, 2025
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.

2 participants