action #162035
opencoordination #58184: [saga][epic][use case] full version control awareness within openQA
coordination #88561: [epic] Extend needle version control handling
Support development of "Use needles from correct ref of NEEDLES_DIR" https://github.com/os-autoinst/openQA/pull/5175 size:M
Description
Motivation¶
It can be confusing to not see candidate needles on the web UI that were not available at the time the test was being executed.
Acceptance criteria¶
- AC1: The candidates view on the test details pages shows candidate needles of the version that was used when the test was executed.
- AC2: Temporary needles are cleaned up (instantly or on a schedule)
- AC3: The needle editor still shows the last candidates. One might optionally show there the candidates at the time the test ran as well.
Suggestions¶
- Read exactly what the PR "Use needles from correct ref of NEEDLES_DIR" is about
- Checkout #154783-10 and further comments for what's missing
- Review the code and make suggestions or add changes yourself to get it merged
- Make sure test coverage is appropriate (PR does not look like it atm.)
- If any work becomes too much work then split out into separate explicit tickets and provide explicit feedback to the PR author about what is expected from them and what can be expected from us
- Saying no to a feature is always an option too
- Consider a NEEDLES_DIR being a different repo/remote entirely (e.g. a pull request) (although this is not strictly required it would be good to ensure we could also support it)
- Read #56789 again
Updated by okurz 8 months ago
- Subject changed from Support development of https://github.com/os-autoinst/openQA/pull/5175 to Support development of "Use needles from correct ref of NEEDLES_DIR" https://github.com/os-autoinst/openQA/pull/5175 size:M
- Description updated (diff)
- Status changed from New to Workable
Updated by openqa_review 6 months ago
- Due date set to 2024-09-03
Setting due date based on mean cycle time of SUSE QE Tools
Updated by tinita 6 months ago
- Status changed from In Progress to Feedback
I tried out the pull request locally and added some notes: https://github.com/os-autoinst/openQA/pull/5175
Still need to understand all of it...
Updated by livdywan 6 months ago
- Related to action #56789: New needles from git repository not working with openqa-clone-custom-git-refspec added
Updated by tinita 6 months ago
- Related to action #157159: Show version of candidate needles the test ran on in the "diff view" on the test details page added
Updated by livdywan 5 months ago
- Status changed from Blocked to Workable
Scott confirmed the AC's we came up with but seems to have no time to work on this for now. The PR has some conflicts. Maybe this can be considered unblocked.
Updated by ybonatakis 5 months ago
Let's wait for Liv to return from vacation next week!?!
Updated by openqa_review 4 months ago
- Due date set to 2024-11-05
Setting due date based on mean cycle time of SUSE QE Tools
Updated by livdywan 4 months ago
Discussed in the unblock
- https://github.com/os-autoinst/openQA/pull/5175 rebased
- I dropped the use of File::Touch which we don't depend on (I was confused by the CI results but the fix was straightforward)
- Looking into coverage gaps which are now also visible on the PR
- Most importantly the candidates view
- Needle editing looks to be using the new code, too, but is not required
- Check how to reproduce the new feature manually e.g. using NEEDLES_DIR/other test vars (same in unit tests)
Updated by gpuliti about 2 months ago
I've created a rebased version of the pr opened by @livdywan https://github.com/os-autoinst/openQA/pull/6097
Updated by livdywan 18 days ago · Edited
- Status changed from Workable to In Progress
- Assignee set to livdywan
Looking to get @gpuliti's changes into the original branch and checking coverage since that was incomplete before.
- [ ] Ensure that we have 100% coverage
- [ ] finalize the wording used in the config file "correct version" vs branch/commit/sha
- [ ] Clarify if @mkittler's comment was addressed regarding "fetched by the web UI"
Updated by openqa_review 17 days ago
- Due date set to 2025-02-18
Setting due date based on mean cycle time of SUSE QE Tools
Updated by livdywan 11 days ago
- [ ] Ensure that we have 100% coverage
- [x] finalize the wording used in the config file "correct version" vs branch/commit/sha
- [ ] Clarify if @mkittler's comment was addressed regarding "fetched by the web UI"
1/3 done, tests currently being added and reviewed
Updated by gpuliti 10 days ago
[ ] Ensure that we have 100% coverage
[x] finalize the wording used in the config file "correct version" vs branch/commit/sha
[x] Clarify if @mkittler's comment was addressed regarding "fetched by the web UI"
With the commit fd4e2469 there are only 3 missing lines to cover that can be seen in codecov. I've already created the test to cover them here, but I think I'm missing the correct endpoint api.
Updated by livdywan 2 days ago
- Copied to action #177504: Conduct lessons learned "Five Why" analysis for "Use needles from correct ref of NEEDLES_DIR" size:S added
Updated by livdywan 2 days ago
Reviewing the current state of the PR
- _create_tmpdir_for_needles_refspec performs network access every time it is called
- git fetch
- vars.json is read but never saved
- there is currently no feature flag for this
- consider "our" ticket #157159 about candidate needles in the diff view
- maybe this PR should exclude the needle editor
- revert changes to the
edit
function
- revert changes to the
- review the current state of tests for the needle editor that would cover the case here
Updated by dheidler 2 days ago · Edited
livdywan wrote in #note-39:
Reviewing the current state of the PR
- _create_tmpdir_for_needles_refspec performs network access every time it is called
- git fetch
That is already adressed but I haven't pushed it yet
- vars.json is read but never saved
Apart from the worker saving it.
It would be much more efficient if we could write that fields back to the database on job finish,
so that we don't need to parse a json file for each needle.
That should be done on job finish, though - maybe with a whitelist of job settings that
should be written from vars.json back to the database.
- there is currently no feature flag for this
What is [scm git]->checkout_needles_sha in the config then?
Updated by livdywan 2 days ago
- Copied to action #177564: Ensure test coverage of needle editor size:S added