Skip to content

Add list and prune commands (simplified version)#253

Merged
florentsfl merged 3 commits into
masterfrom
fallard/add-clean-images-command
Jan 20, 2026
Merged

Add list and prune commands (simplified version)#253
florentsfl merged 3 commits into
masterfrom
fallard/add-clean-images-command

Conversation

@florentsfl

Copy link
Copy Markdown
Contributor

this is a potential alternative to #251 which does not keep a cache of the running images but cleans every image which is starting by cqfd_$USER on the host

@florentsfl
florentsfl force-pushed the fallard/add-clean-images-command branch 2 times, most recently from d674890 to 3591add Compare January 9, 2026 16:04

@deribaucourt deribaucourt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice feature!

Comment thread cqfd
Comment thread cqfd Outdated
Comment thread cqfd.1.adoc Outdated
Comment thread tests/99-teardown.bats
_common_setup
}

@test "'cqfd images' lists the build containers" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if bats tries to run these tests in parallel, they won't work.
Put all of these in a sequential test instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually bats is running the tests sequentially, do you think I should make this explicit ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bats has some arguments to select which tests to run, so it would also break the sequence.
Even though today we always run all of them, sequentially, in the CI, I think it would be a better practice to group sequential dependencies in a single test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried to parallelize tests in another PR #255 but it has a lot more issues than this one, and I think that it would be part of a refactor of the tests but it does not add a lot of value compared to the current system.

So I think I will leave it like that to be consistent with the other tests, and I may try to parallelize the tests in another PR at some point

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not putting all the code in one single test? That way we ensure it is run sequentially.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we lose atomicity in the tests. It is useful to know which command makes a test fail.

If we have 4 different tested commands in one @test, and if one of them fail we won’t know which one makes the test fail. Having four different tests makes it possible to know which exact command fail.

If we want the tests to run sequentially, then we can add export BATS_NO_PARALLELIZE_WITHIN_FILE=true to the setup_file function

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would argue that bats would still report which assert failed, so you know exactly what failed. And anyway, if one step failed, the following ones would also fail, so you can only get a more confusing output by having a split test IMO.

BATS_NO_PARALLELIZE_WITHIN_FILE does sound like an alternative improvement also.

Anyway, the CI doesn't care since it always runs everything and does so sequentially, so I'll approve the PR this way. I think we spent enough time on a problem that we probably won't face.

@florentsfl
florentsfl force-pushed the fallard/add-clean-images-command branch 3 times, most recently from f990dd6 to 1d625d1 Compare January 16, 2026 20:10
gportay and others added 3 commits January 16, 2026 15:34
@florentsfl
florentsfl force-pushed the fallard/add-clean-images-command branch from 1d625d1 to 9ec8391 Compare January 16, 2026 20:35
@florentsfl
florentsfl merged commit df206c5 into master Jan 20, 2026
2 checks passed
@florentsfl
florentsfl deleted the fallard/add-clean-images-command branch January 20, 2026 21:19
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.

4 participants