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

test(NFS): run either nfsv4 or nfsv3 tests #2456

Closed
wants to merge 1 commit into from

Conversation

Henrik66
Copy link
Contributor

@Henrik66 Henrik66 commented Jul 29, 2023

Run only nfsv4 tests unless nfs4 is not available

Checklist

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

Fixes #2455

@github-actions github-actions bot added the test Issues related to testing label Jul 29, 2023
Copy link
Collaborator

@LaszloGombos LaszloGombos left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks. This helps having passing CI without compromising test coverage significantly.

@LaszloGombos LaszloGombos added this to the dracut-060 milestone Jul 29, 2023
test_nfsv3 \
&& test_nfsv4
# run either nfsv4 or nfsv3 tests
if grep -q '\-nfs4' /etc/portage/package.use/nfs-utils 2>/dev/null; then
Copy link
Member

Choose a reason for hiding this comment

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

isn't this check only for gentoo?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this check would only pass on Gentoo. In practice only the Gentoo container does not support NFSv4, so only Gentoo would run the NFSv3 tests by default.

I am not aware of how to test for this in a non-distribution specific way.

Perhaps one alternative is to introduce a command line option for tests and pass in in the test run command line if we only want to run NFSv3 or NFSv4 tests (or both).

@LaszloGombos
Copy link
Collaborator

This is a discussion meant to focus on what is running on CI (and not what can or should be tested outside of CI).
Currently Test 20 is longest running and most likely to timeout test on our CI (with 45 min timeout). Only enabled on Fedora.

To get to a state where CI is green and/or we enable this test for other distributions also, we need to either:

  • make the test do less things and finish earlier (this is what this current PR proposes)
  • increase timeout for the test (not a good option)
  • parallelize subtests instead of running them sequentially (will take a while to come up with a way to do so)

CC @bdrung for visibility and feedback.

@bdrung what do you think of the goal of this PR and/or the way it is trying to do it ? Is there a better way to detect installed NFS versions in a distribution agonistic way ?

@bdrung
Copy link
Contributor

bdrung commented Aug 24, 2023

The first question to answer is: Do we want to run only NFS v3 or v4 tests? How widely used is NFS v3 vs v4? If v3 is legacy and most of all systems use v4, I am fine with only running v4 tests (if v4 support is available). Other having tested both would be preferred.

@LaszloGombos
Copy link
Collaborator

The first question to answer is: Do we want to run only NFS v3 or v4 tests?
The current Gentoo test container only supports NFS v3. All other test containers support NFS v4.

I am fine with only running v4 tests (if v4 support is available). Other having tested both would be preferred.
Me too - this is what this PR is proposing already.

@LaszloGombos LaszloGombos modified the milestones: dracut-060, dracut-061 Oct 30, 2023
@aafeijoo-suse aafeijoo-suse removed this from the dracut-061 milestone Nov 18, 2023
@Henrik66 Henrik66 closed this Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

modernize nfs testing
4 participants