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

Add repeat parameter to clone a job multiple times #5331

Merged
merged 1 commit into from Oct 19, 2023

Conversation

ilmanzo
Copy link
Contributor

@ilmanzo ilmanzo commented Oct 16, 2023

Reference ticket: https://progress.opensuse.org/issues/122953

to better support statistical investigation, we want to provide an
immediate way to clone a job multiple times. Till now it has been done
with a for loop similar to

for i in {01..50}; do openqa-clone-job … TEST+=-$i; done

But we can avoid the API overhead of copying the same parameters and
assets again and again from the original job, simply by cloning it once and
posting the new job(s) N times. The TEST name is automatically numbered, so the
user is not forced to remember any shell syntax.

As the default is --repeat=1, the API remains compatible with any shell
script using the openqa-clone-job program.

@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching docs/*.asciidoc:

  • Consider generating documentation locally to verify it is rendered correctly using tools/generate-htmldoc

This is an automatically generated QA checklist based on modified files.

@ilmanzo ilmanzo force-pushed the poo122953_multiple_jobs branch 2 times, most recently from ae93e46 to 25b4390 Compare October 16, 2023 09:43
lib/OpenQA/Script/CloneJob.pm Outdated Show resolved Hide resolved
lib/OpenQA/Script/CloneJob.pm Outdated Show resolved Hide resolved
@Martchus
Copy link
Contributor

I suppose this feature deserves a unit test.

@Martchus
Copy link
Contributor

Martchus commented Oct 16, 2023

By the way, what prevents you from doing simply for i in {01..50}; do openqa-clone-job … TEST+=-$i; done? I always use a command similar to that and I'm sure people on the team sometimes do the same.

@ilmanzo
Copy link
Contributor Author

ilmanzo commented Oct 16, 2023

By the way, what prevents you from doing simply for i in {01..50}; do openqa-clone-job … TEST+=-$i; done? I always use a command similar to that and I'm sure people on the team sometimes do the same.

No one prevents you, I guess it's just a matter of 'user experience', as discussed in the ticket. You can always wrap the clone-job inside a for loop, but if you feel lazy just use the --repeat parameter .. Or do both :)

@ilmanzo ilmanzo marked this pull request as draft October 16, 2023 10:13
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (f46075b) 98.32% compared to head (5667ab6) 98.32%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5331   +/-   ##
=======================================
  Coverage   98.32%   98.32%           
=======================================
  Files         389      389           
  Lines       37286    37317   +31     
=======================================
+ Hits        36660    36691   +31     
  Misses        626      626           
Files Coverage Δ
lib/OpenQA/Script/CloneJob.pm 97.31% <100.00%> (+0.16%) ⬆️
script/openqa-clone-job 83.78% <100.00%> (+1.43%) ⬆️
t/35-script_clone_job.t 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@foursixnine
Copy link
Member

Looks pretty good

Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

Sounds like a nice idea but please provide much more detail into your git commit message to be able to follow your motivation and design choices

docs/UsersGuide.asciidoc Outdated Show resolved Hide resolved
docs/UsersGuide.asciidoc Outdated Show resolved Hide resolved
lib/OpenQA/Script/CloneJob.pm Outdated Show resolved Hide resolved
script/openqa-clone-job Outdated Show resolved Hide resolved
@okurz
Copy link
Member

okurz commented Oct 17, 2023

I saw that you updated your PR but the suggestions from comments haven't been implemented yet. Can you implement those or comment on which different path you want to take?

@ilmanzo ilmanzo marked this pull request as ready for review October 17, 2023 09:54
@ilmanzo
Copy link
Contributor Author

ilmanzo commented Oct 17, 2023

I suppose this feature deserves a unit test.

Agree; added a new unit test

script/openqa-clone-job Outdated Show resolved Hide resolved
lib/OpenQA/Script/CloneJob.pm Outdated Show resolved Hide resolved
script/openqa-clone-job Outdated Show resolved Hide resolved
script/openqa-clone-job Outdated Show resolved Hide resolved
@ilmanzo ilmanzo requested a review from okurz October 19, 2023 07:17
@okurz
Copy link
Member

okurz commented Oct 19, 2023

From https://app.circleci.com/pipelines/github/os-autoinst/openQA/12328/workflows/dec7ae53-201a-46cc-9558-0a06c041ce8a/jobs/115056 it says "Cloning children of myjob". Please make sure that all output is caught by Test::Output functions or other means. Possibly this test or others already provide good examples.

to better support statistical investigation, we want to provide an
immediate way to clone a job multiple times. Till now it has been done
with a for loop similar to

for i in {01..50}; do openqa-clone-job … TEST+=-$i; done

But we can avoid the API overhead of copying the same parameters and
assets again and again from the original job, simply by cloning it once and
posting the new job(s) N times. TEST name is automatically numbered, so the
user is not forced to remember any shell syntax.

As the default is --repeat=1, the API remains compatible with any shell
script using the openqa-clone-job program.
@ilmanzo
Copy link
Contributor Author

ilmanzo commented Oct 19, 2023

From https://app.circleci.com/pipelines/github/os-autoinst/openQA/12328/workflows/dec7ae53-201a-46cc-9558-0a06c041ce8a/jobs/115056 it says "Cloning children of myjob". Please make sure that all output is caught by Test::Output functions or other means. Possibly this test or others already provide good examples.

good point, fixed with latest commit-push

@okurz
Copy link
Member

okurz commented Oct 19, 2023

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Oct 19, 2023

rebase

✅ Nothing to do for rebase action

@perlpunk
Copy link
Contributor

https://app.circleci.com/pipelines/github/os-autoinst/openQA/12338/workflows/3f019372-4b18-4b49-8747-2e4279b06b83/jobs/115171

12:19:11] t/33-developer_mode.t .. 17/?     # Unexpected Javascript console errors, waiting for connection opened: [
    #   {
    #     level     => "SEVERE",
    #     message   => "http://localhost:9526/asset/285cd7a70d/ws_console.js 8 WebSocket connection to 'ws://localhost:9528/liveviewhandler/tests/1/developer/ws-proxy' failed: Error during WebSocket handshake: Unexpected response code: 302",
    #     source    => "network",
    #     timestamp => 1697718002718,
    #   },
    # ]

    #   Failed test 'No unexpected js warnings'
    #   at /home/squamata/project/t/lib/OpenQA/Test/FullstackUtils.pm line 123.
    # Looks like you failed 1 test of 9.
[12:19:11] t/33-developer_mode.t .. 20/? 
#   Failed test 'resume test execution and 2nd tab'
#   at t/33-developer_mode.t line 340.
[12:19:11] t/33-developer_mode.t .. 23/? # Looks like you failed 1 test of 23.

I guess that's unrelated. I will retrigger the test.

@mergify mergify bot merged commit c5419ae into os-autoinst:master Oct 19, 2023
36 checks passed
@okurz
Copy link
Member

okurz commented Oct 19, 2023

thank you. With the rebase request my intent was the sam but there was nothing to rebase so that did not retrigger anything

os-autoinst-bot pushed a commit to os-autoinst-bot/openQA that referenced this pull request Oct 20, 2023
commit c5419ae
Merge: f721830 5667ab6
Author:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AuthorDate: Thu Oct 19 15:47:00 2023 +0000
Commit:     GitHub <noreply@github.com>
CommitDate: Thu Oct 19 15:47:00 2023 +0000

    Merge pull request os-autoinst#5331 from ilmanzo/poo122953_multiple_jobs

    Add repeat parameter to clone a job multiple times
@ilmanzo ilmanzo deleted the poo122953_multiple_jobs branch October 20, 2023 06:13
@Martchus
Copy link
Contributor

I've just used this parameter to create https://openqa.suse.de/tests/overview?version=5.5&build=Martchus%2Fos-autoinst-distri-opensuse%2318034-m2048&distri=sle-micro and admittedly, it is more convenient than the for loop :-)

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

6 participants