Skip to content

openqa-bats-review: Update expected outside of loop#523

Merged
mergify[bot] merged 2 commits intoos-autoinst:masterfrom
ricardobranco777:noloop
Feb 10, 2026
Merged

openqa-bats-review: Update expected outside of loop#523
mergify[bot] merged 2 commits intoos-autoinst:masterfrom
ricardobranco777:noloop

Conversation

@ricardobranco777
Copy link
Contributor

@ricardobranco777 ricardobranco777 commented Feb 10, 2026

We were updating the expected dict when iterating over the job list!

We should do this once and it's safe to do so because get_job() is cached.

Also enhance the logging that helped catch this.

Otherwise we get:

$ openqa-bats-review -n https://openqa.opensuse.org/tests/5662790
INFO: Processing clone chain: 5662790 -> 5662653 -> 5662067 -> 5661930 -> 5661406
INFO: Job 5662653 has only 4 logs (expected: 6), skipping
INFO: Job 5662067 has only 4 logs (expected: 8), skipping
INFO: Job 5661930 has only 4 logs (expected: 10), skipping
INFO: Job 5661406 has only 4 logs (expected: 12), skipping

Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

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

I don't care about this particular script but in general it is good to state the intention of the change in the commit message title.

@ricardobranco777
Copy link
Contributor Author

I don't care about this particular script but in general it is good to state the intention of the change in the commit message title.

It's a fix and I think the current commit message states the intention.

We were updating the expected dict when iterating on the job list.
We should do this once and it's safe to do so because get_job()
is cached.
@mergify mergify bot merged commit ab6e1e3 into os-autoinst:master Feb 10, 2026
8 checks passed
@ricardobranco777 ricardobranco777 deleted the noloop branch February 10, 2026 19:44
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.

4 participants