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: invalid behavior on sid/alternative in 5.3.4/99.5.4.5.1 #237

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

sinrazon
Copy link
Contributor

On Debian Sid / alternative distributions (such as Ubuntu), these checks would fail with sid: integer expression expected, which is to be expected since $DEB_MAJ_VER is set to a string, not an integer.

This small PR fixes that in a similar fashion to other checks in the source tree.

@sinrazon sinrazon force-pushed the sinrazon/fix_sid_checks branch 3 times, most recently from bb12b44 to b9d625b Compare February 28, 2024 17:04
@sinrazon sinrazon changed the title fix: invalid behavior on sid/alternative in 5.4.5.1/99.5.4.5.1 fix: invalid behavior on sid/alternative in 5.3.4/99.5.4.5.1 Feb 28, 2024
@ThibaultDewailly
Copy link
Collaborator

Hello and welcome to this repository !

Please code the tests associated to the Pull request, in order to validate the behavior you are introducing

@ThibaultDewailly ThibaultDewailly self-assigned this Mar 5, 2024
Copy link
Collaborator

@ThibaultDewailly ThibaultDewailly left a comment

Choose a reason for hiding this comment

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

Blocker : Associated tests are mandatory

@sinrazon
Copy link
Contributor Author

sinrazon commented Apr 9, 2024

As requested, I've reworked this PR to include related tests.

A few notes about these changes :

  • @ThibaultDewailly suggested inverting the $DEB_MAJ_VER condition, to check for Debian 11 first, then sid; however, doing so does not fix the aforementioned error (string being tested against an integer), so this change was not made.
  • There is no test utility to "mock" a Debian version, and the $DEB_MAJ_VER utility is outside of the individual test scope, which is not overridable. The accepted solution is to monkey-patch /etc/debian_version on the fly.
  • A lot of tests were missing for both of these checks, so I've added them based on similar checks + new tests for Debian Sid.
  • A couple of tweaks were needed to make the apply() function work for these scripts. I think the lack of sufficient testing broke them a while ago.

All these changes were tested against Debian 10/11/12.

Please let me know if anything more is needed.

@sinrazon sinrazon force-pushed the sinrazon/fix_sid_checks branch 2 times, most recently from 1a8ca38 to 49348af Compare April 9, 2024 14:35
@ThibaultDewailly ThibaultDewailly merged commit 6079b16 into ovh:master Apr 9, 2024
5 checks passed
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.

None yet

2 participants