Fix cryptic unit parsing error in Data Size Fields
This mr partially fixes #4273, the tmt run part, but not the tmt lint part, cause when users run "tmt lint", normalize_* are not called, despite repeated confirmation from Claude and TMT Code Buddy that they were being called.Thank Milos for quickly identifying this issue after several minutes' investigation, more mr(s) will be sent.
Pull Request Checklist
- [x] implement the feature
- [ ] write the documentation
- [ ] extend the test coverage
- [ ] update the specification
- [ ] adjust plugin docstring
- [ ] modify the json schema
- [ ] mention the version
- [ ] include a release note
LGTM. My only comment is that the
normalize_*should be generalized to reduce duplication, but that's for a different PR.
haha, Milos has the same concern, gonna to send a mr to address that.
LGTM. My only comment is that the
normalize_*should be generalized to reduce duplication, but that's for a different PR.haha, Milos has the same concern, gonna to send a mr to address that.
I'd maybe switch the order of these PRs, because here you are adding code the next PR will remove. Change the order, and will not need to remove code that has not been added because those two functions no longer contain their copy of code, but use a shared helper instead.
LGTM. My only comment is that the
normalize_*should be generalized to reduce duplication, but that's for a different PR.haha, Milos has the same concern, gonna to send a mr to address that.
I'd maybe switch the order of these PRs, because here you are adding code the next PR will remove. Change the order, and will not need to remove code that has not been added because those two functions no longer contain their copy of code, but use a shared helper instead.
addressed :sweat_drops:
@skycastlelily a test, even a small one, and a release note would be nice.
Please make a comment or reset the reviews and link the relevant dependent PR when you update the branch since Github doesn't seem to notify about that change (I think it does if it is automatically moved due to a PR being merged).
Sure :)
LGTM, about the testing, it would be really good to add it, especially since here we care about the order of when this validation fails as per the related issue.
addressed: https://github.com/teemtee/tmt/pull/4285/commits/6dbc4ede0093c71be59e11f7967383b74c5ba353
@skycastlelily a test, even a small one, and a release note would be nice.
Sure, https://github.com/teemtee/tmt/pull/4285/commits/6dbc4ede0093c71be59e11f7967383b74c5ba353 ^^
Regarding the tests, good to have unit test coverage, but it doesn't really cover the breakage of #4273. For that one we need an explicit tmt test somewhere in
/tests/provision/hardwarethat checks that we exit as early as possible and we do not enter even thediscoverphase.
Addressed https://github.com/teemtee/tmt/commit/c90dcf8ae8ebc5a18d5f7dc6cf1a6f15204d3f35 :)
@skycastlelily, please update the pull request checklist and move the release note to the docs/releases/pending/ folder. Thanks.
@skycastlelily, please update the pull request checklist and move the release note to the
docs/releases/pending/folder. Thanks.
sure,and done.
@skycastlelily please, inspect the tests, there seems to be a problem with PintError.
@skycastlelily please, inspect the tests, there seems to be a problem with
PintError.
hmmm,
cannot import name 'PintError' from 'pint.errors' (/usr/lib/python3.9/site-packages/pint/errors.py)" error,
because PintError does not exit in python3.9, should we replace PintError with Union[UndefinedUnitError, DimensionalityError]?:)
@skycastlelily please, inspect the tests, there seems to be a problem with
PintError.hmmm,
cannot import name 'PintError' from 'pint.errors' (/usr/lib/python3.9/site-packages/pint/errors.py)" error, because PintError does not exit in python3.9, should we replace PintError with Union[UndefinedUnitError, DimensionalityError]?:)
That is quite unfortunate, especially since it seems we are the only consumers of python-pint and there is no reason to not update it
$ fedrq wrsrc python-pint -b epel9 -F source
tmt
In the meantime, I think it's fine to just put it as except Exception in those places instead
hmmm,
cannot import name 'PintError' from 'pint.errors' (/usr/lib/python3.9/site-packages/pint/errors.py)" error, because PintError does not exit in python3.9, should we replace PintError with Union[UndefinedUnitError, DimensionalityError]?:)That is quite unfortunate, especially since it seems we are the only consumers of python-pint and there is no reason to not update it
$ fedrq wrsrc python-pint -b epel9 -F source tmtIn the meantime, I think it's fine to just put it as
except Exceptionin those places instead
Thanks ,updated :)
/packit test --id provision:virtual
/packit build