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

Prevent override the autoinst-log context within handle_unreachable #347

Merged
merged 8 commits into from
Nov 27, 2024

Conversation

b10n1k
Copy link
Contributor

@b10n1k b10n1k commented Sep 15, 2024

handle_unreachable can override the log file passed to the function. This should not allowed because when the handle_unreviewed function is reach will run with incorrect logs. Solving the issue creating a separate temp local file
for handle_unreachable and extract timeago in a variable in order to use it to
determine the age of the job. This keeps clear now the out variable which
should only contain the autoinst-log+reason.

https://progress.opensuse.org/issues/166772

@b10n1k b10n1k force-pushed the label_known_issues_override branch 3 times, most recently from 43c7ebc to 348e95e Compare September 15, 2024 19:26
@b10n1k b10n1k force-pushed the label_known_issues_override branch from 348e95e to a3539b4 Compare November 13, 2024 08:01
@b10n1k b10n1k requested a review from perlpunk November 13, 2024 08:01
@b10n1k b10n1k force-pushed the label_known_issues_override branch 2 times, most recently from a05283a to 3ddb915 Compare November 13, 2024 09:23
openqa-label-known-issues Outdated Show resolved Hide resolved
@b10n1k b10n1k force-pushed the label_known_issues_override branch from 3ddb915 to fcecd1c Compare November 13, 2024 13:32
@b10n1k b10n1k marked this pull request as ready for review November 13, 2024 13:39
openqa-label-known-issues Outdated Show resolved Hide resolved
openqa-label-known-issues Outdated Show resolved Hide resolved
@b10n1k b10n1k force-pushed the label_known_issues_override branch from fcecd1c to b2a8159 Compare November 14, 2024 08:21
@b10n1k b10n1k requested a review from perlpunk November 14, 2024 08:23
@b10n1k b10n1k force-pushed the label_known_issues_override branch from b2a8159 to af67a70 Compare November 14, 2024 08:29
@b10n1k b10n1k force-pushed the label_known_issues_override branch from af67a70 to 73aed5c Compare November 15, 2024 11:40
openqa-label-known-issues Outdated Show resolved Hide resolved
Copy link
Contributor

@perlpunk perlpunk left a comment

Choose a reason for hiding this comment

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

hxselect is overwritten in the test with just cat -, so it won't return the date. That's why the test fails

@b10n1k b10n1k force-pushed the label_known_issues_override branch from 73aed5c to 6ac7a1f Compare November 25, 2024 09:24
test/07-openqa-label-known-issues.t Outdated Show resolved Hide resolved
@b10n1k b10n1k force-pushed the label_known_issues_override branch 2 times, most recently from d872396 to 0f59bc3 Compare November 25, 2024 13:47
@b10n1k b10n1k requested review from Martchus and perlpunk November 25, 2024 23:20
openqa-label-known-issues Outdated Show resolved Hide resolved
@perlpunk perlpunk force-pushed the label_known_issues_override branch from b113b67 to 38ba8dc Compare November 26, 2024 11:08
@perlpunk
Copy link
Contributor

I pushed a commit.
Can you now please check the tests?

test/07-openqa-label-known-issues.t Outdated Show resolved Hide resolved
@b10n1k b10n1k force-pushed the label_known_issues_override branch 4 times, most recently from 6dd1177 to f5bcce6 Compare November 26, 2024 12:51
@b10n1k b10n1k requested a review from perlpunk November 26, 2024 19:08
b10n1k and others added 4 commits November 27, 2024 11:26
`handle_unreachable` can override the log file passed to the function. This
should not allowed because when the `handle_unreviewed` function is reach will
run with incorrect logs. Solving the issue creating a separate temp local file
for handle_unreachable and extract timeago in a variable in order to use it to
determine the age of the job. This keeps clear now the out variable which
should only contain the autoinst-log+reason.

https://progress.opensuse.org/issues/166772

Signed-off-by: ybonatakis <[email protected]>
Signed-off-by: Ioannis Bonatakis <[email protected]>
@perlpunk perlpunk force-pushed the label_known_issues_override branch from f5bcce6 to 47056a1 Compare November 27, 2024 10:30
    /home/runner/work/scripts/scripts/.bpan/.bpan/lib/test-tap.bash: ERROR: line 168

Tests pass, it's just printing that error.

I'm not sure why that's needed here as `try` should take care of it, but
I think it's because the error for calling investigate_issue without
an argument is "Unbound variable" and that might not be catched the same way as
other error messages. I might ask the test-tap author about it.
and move creation of temp files to top level (this only needs to be done once)
and pass the correct variable. investigate_issue gets a url as a parameter
and then strips everything until the last slash to get the id.
Apparently it wasn't too broken for the test to fail though
@perlpunk perlpunk force-pushed the label_known_issues_override branch from 02626cd to eabd76b Compare November 27, 2024 11:22
@perlpunk perlpunk dismissed their stale review November 27, 2024 11:23

fixed it

@mergify mergify bot merged commit e8d8e56 into os-autoinst:master Nov 27, 2024
4 checks passed
@perlpunk
Copy link
Contributor

I added some more fixes myself

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