Skip to content

testvol: return error in Path endpoint when volume is unmounted - #29302

Open
prayag0one4 wants to merge 1 commit into
podman-container-tools:mainfrom
prayag0one4:fix-testvol-mount-check
Open

testvol: return error in Path endpoint when volume is unmounted#29302
prayag0one4 wants to merge 1 commit into
podman-container-tools:mainfrom
prayag0one4:fix-testvol-mount-check

Conversation

@prayag0one4

@prayag0one4 prayag0one4 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

This updates the testvol mock driver's Path() endpoint to verify if a volume is actually mounted before returning a path. If the volume has no active mounts, it now correctly returns an error instead of a fake path, resolving the existing TODO in test/testvol/main.go.

Copilot AI review requested due to automatic review settings July 26, 2026 19:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mheon

mheon commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Sure, LGTM

@Luap99 Are we running unit tests in the test/ directory?

@mheon mheon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Luap99

Luap99 commented Jul 27, 2026

Copy link
Copy Markdown
Member

@Luap99 Are we running unit tests in the test/ directory?

I think so but we would need to check the logs.

@Luap99 Luap99 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.

Note changing the content of the test/testvol does nothing since there is no automation to publish the new image.

A maintainer with push access would need to build/publish a new image with make volume-plugin-test-img and push that and then update the image reference here to actually verify this passes e2e tests, until this happens it cannot be merged

Also please see https://github.com/podman-container-tools/podman/blob/main/LLM_POLICY.md
PR descriptions like that are useless, I can see what files were changed by looking at the diff.

Comment thread test/testvol/main_test.go Outdated
"github.com/docker/go-plugins-helpers/volume"
)

func TestPathUnmountedVolume(t *testing.T) {

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 don't see the point of the test tbh, this is already a test only code which is only run in e2e.

@prayag0one4

Copy link
Copy Markdown
Contributor Author

Thanks for review @Luap99 !
I have simplified the PR description and removed main_test.go Since it is test-only code. Understood regarding the volume-plugin-test-img build process!

@prayag0one4
prayag0one4 requested a review from Luap99 July 27, 2026 19:28
@TomSweeneyRedHat

Copy link
Copy Markdown
Contributor

LGTM
@Luap99 do the changes work for you?

@mheon

mheon commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Please squash your commits and we can merge

Check if volume has active mounts in Path() endpoint and return an error if unmounted, resolving TODO.

Signed-off-by: Prayag <rajprayag014@gmail.com>
@prayag0one4
prayag0one4 force-pushed the fix-testvol-mount-check branch from 8a4a8c8 to 72f7cc4 Compare August 7, 2026 05:30
@prayag0one4

Copy link
Copy Markdown
Contributor Author

squashed into single commit.

@mheon

mheon commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

LGTM
@Luap99 PTAL

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.

5 participants