Skip to content

test: resolve flaky TestGlobFiles by ignoring filesystem ordering#1849

Closed
maishivamhoo123 wants to merge 1 commit into
NVIDIA:mainfrom
maishivamhoo123:fix-flaky-test
Closed

test: resolve flaky TestGlobFiles by ignoring filesystem ordering#1849
maishivamhoo123 wants to merge 1 commit into
NVIDIA:mainfrom
maishivamhoo123:fix-flaky-test

Conversation

@maishivamhoo123
Copy link
Copy Markdown

I looked into this, and the issue is a flaky test caused by filesystem ordering.

Operating systems and filesystems (like those typically used on Arch Linux) don't guarantee the order in which they return files when reading a directory. While the files are correctly found, TestGlobFiles uses require.Equal() at the end of the test, which strictly enforces the array index order, causing the mismatch.

To fix this without altering the actual tool's behavior, we just need to update the test assertion in cmd/nvidia-cdi-hook/cudacompat/container-root_test.go to ignore the order.

Changing require.Equal(t, tc.expected, got) to require.ElementsMatch(t, tc.expected, got) (or sorting got beforehand) resolves the flake.

I have a fix ready and will open a PR for this shortly!
Closes : #1847

Signed-off-by: maishivamhoo123 <maishivamhoo@gmail.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 25, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copy link
Copy Markdown
Contributor

@cdesiniotis cdesiniotis left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @maishivamhoo123. Can you update this so that the change is made to the right test method?

root, _ := newRoot(containerRootDir)
got := root.hasPath(tc.path)
require.Equal(t, tc.expected, got)
require.ElementsMatch(t, tc.expected, got)
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.

@maishivamhoo123 did you mean to make this change in the TestGlobFiles method? Right now this is updating the TestHasPath method.

@cdesiniotis
Copy link
Copy Markdown
Contributor

On further thought, I slightly prefer proceeding with #1846 instead of this PR. Prior to the v1.19.1 release, our internal globfiles() method used filepath.Glob (see here) which does appear to return matches in lexigraphical order (reference).

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]: unit test failure on version 1.19.1

2 participants