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: skip test 50,60 if ifcfg dracut module can not be installed #2543

Closed
wants to merge 1 commit into from

Conversation

Henrik66
Copy link
Contributor

@Henrik66 Henrik66 commented Oct 29, 2023

Test 50 and 60 have a dependency on the ifcfg dracut module.
If ifcfg dracut module is not available, than the preconditions
for these tests are not met.

Since ifcfg dracut module is not available in many Linux enviroments
(such as Arch or Debian), it make sense to explicitelly skip these
tests instead of failing them as they are simply not applicable
to many Linux distributions in their current form.

@github-actions github-actions bot added the test Issues related to testing label Oct 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.

CC @bdrung

@bdrung
Copy link
Contributor

bdrung commented Oct 30, 2023

Please provide a reasoning in the git commit. At least these tests were failing on Debian/Ubuntu (see also bug #2328).

@Henrik66
Copy link
Contributor Author

@bdrung Thanks for the review. I added more reasoning to the git commit message.

test: skip test 50,60 if ifcfg dracut module can not be installed

Test 50 and 60 have a dependency on the ifcfg dracut module.
If ifcfg dracut module is not available, than the preconditions
for these tests are not met.

Since ifcfg dracut module is not available in many Linux enviroments
(such as Arch or Debian), it make sense to explicitelly skip these
tests instead of failing them as they are simply not applicable
to many Linux distributions in their current form.

As to your question, these tests are only meant to run if ifcfg dracut module is supported.
A better solution would be to reimplement these tests so that they do not require ifcfg dracut module.
I am not able to provide such a solution at this time, so until than perhaps we can just skip the test.

@bdrung
Copy link
Contributor

bdrung commented Oct 31, 2023

@bdrung Thanks for the review. I added more reasoning to the git commit message.

Thanks.

As to your question, these tests are only meant to run if ifcfg dracut module is supported. A better solution would be to reimplement these tests so that they do not require ifcfg dracut module. I am not able to provide such a solution at this time, so until than perhaps we can just skip the test.

That information could go into the commit message as well, saying something like: Instead of disabling the test, they should be reimplement so that they do not require ifcfg dracut module in the future.

Test 50 and 60 have a dependency on the ifcfg dracut module.
If ifcfg dracut module is not available, than the preconditions
for these tests are not met.

Since ifcfg dracut module is not available in many Linux enviroments
(such as Arch or Debian), it make sense to explicitelly skip these
tests instead of failing them as they are simply not applicable
to many Linux distributions in their current form.

Instead of disabling the tests, they should be reimplement
so that they do not require ifcfg dracut module in the future.
@Henrik66
Copy link
Contributor Author

That information could go into the commit message as well, saying something like: Instead of disabling the test, they should be reimplement so that they do not require ifcfg dracut module in the future.

okay. I updated the commit message. Thanks @bdrung

@LaszloGombos LaszloGombos added this to the dracut-061 milestone Nov 1, 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.

4 participants