teemtee
Improve template url generations
Depends on #2489
- [ ] 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
@LecrisUT we would love to merge this, would it be a big ask to add some unit tests for the improved checkout logic pls??
Looking at it, seems doable. I guess there aren't unit tests around it to hook to? I'll be incapacitated due to a cold probably until Monday though
Looking at it, seems doable. I guess there aren't unit tests around it to hook to? I'll be incapacitated due to a cold probably until Monday though
I believe this (alongside https://github.com/teemtee/tmt/pull/2489) would be more suitable to be merged after the 1.35 release anyway, wdyt?
Initial unit tests added. Right now commit checkouts and detached merged commits behave the same, haven't checked how to mimic Github's refs/remotes/pull/N/merge. Also I haven't covered fmf_id.url and other remote logic.
The linkcheck might fail with 404 if it points to a commit that is only available on a fork.
lgtm, but not going to pretend I understand it all :)
@thrix @happz How does it look to you now, with the unit test added? Do we need full test?
I don't understand the changes either, that's why I'm hoping smarter reviewers would take a look... But I think we do need full tests in any case.
To try to summarize:
- Original
git rev-parse --abbrev-ref --symbolic-full-name @{u}did not work on detached heads for tags and detached merges (github's action/checkout) - Instead of that, it is trying to detect the most human-readable commitish ref:
branch->tag->commit. - I was hoping to add other refs like extracting the PR link, but that doesn't seem to work, and at least the detached merge commits do not seem to 404, so I guess it's fine like this (example)
One quick note, I am working on moving this git ref resolution logic to tmt.utils.git and making it cached according to tmt_root.
Edit: aand done.
Re-running failed tests, looks mostly flakes, plus Fedora 41 is now on TF. Hopefully we do not hit:
https://status.testing-farm.io/issues/2024-08-02-fedora-40-small-disk-size/
Ah, no, I had an error there:
:: [ 12:13:25 ] :: [ BEGIN ] :: Running 'expected_ref=$(git describe --tags eae4d527)'
fatal: No names found, cannot describe anything.
:: [ 12:13:25 ] :: [ FAIL ] :: Command 'expected_ref=$(git describe --tags eae4d527)' (Expected 0, got 128)
Forgot that part is not in a git environment
@LecrisUT, would mind adding a short release note about the change?
I'll try to think of some words tomorrow. I guess the part that should be mentioned is when it is run with fmf show or debug flags in imported plan/test. Shouldn't affect the actual ref used, just the displayed ref in fmf_id.
A bit unfortunate that PyCharm does not support #: docstrings ^1, but that's a topic for a different PR. @psss what do you think of the wording for the release notes?
@psss what do you think of the wording for the release notes?
The release note looks great, thanks!