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

Fix Linux version regex #1852

Merged
merged 1 commit into from
Aug 10, 2022
Merged

Fix Linux version regex #1852

merged 1 commit into from
Aug 10, 2022

Conversation

kelszo
Copy link
Contributor

@kelszo kelszo commented Aug 10, 2022

Closes #1851

Current PR fixes an incorrect regex for detecting Linux kernel versions leading to not being able to create a Linux virtual package on certain distros.

@jonashaag
Copy link
Contributor

Thanks for the patch. Can you add a test?

@kelszo
Copy link
Contributor Author

kelszo commented Aug 10, 2022

Shouldn't the patch be covered by the test_virtual_packages.cpp tests already? Or do you want more specific tests?

@wolfv wolfv merged commit 121c348 into mamba-org:master Aug 10, 2022
@jonashaag
Copy link
Contributor

Shouldn't the patch be covered by the test_virtual_packages.cpp tests already? Or do you want more specific tests?

If we had a test for this change then CI would’ve been red before, but it was green, or do I miss something?

@kelszo
Copy link
Contributor Author

kelszo commented Aug 11, 2022

The reason why the tests didn't fail before was because in the CI they seem to run on Ubuntu. Ubuntu's kernel versions include the flavour tag. See #1851 for more information.

The tests failed first locally on my machine (NixOS) so I guess running the tests on such a distro would cover this patch.

@jonashaag
Copy link
Contributor

jonashaag commented Aug 12, 2022

Thanks, that makes sense. I guess it’s not worth adding another distro to the CI setup but we could add a unit test for the code that matches the regex and supply a hardcoded name in the test.

@jonashaag
Copy link
Contributor

It would require that we refactor the code a little bit though because as it is it’s not testable unless we override the uname program

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.

Incorrect Linux version regex [linux version not found (virtual package skipped)]
3 participants