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

[RHELC-788] Implement integration test error after ponr #1265

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

SerCantus
Copy link
Member

@SerCantus SerCantus commented Jun 18, 2024

Implement a new integration test to verify the log message when a fail occurs after PONR

Jira Issues:

Checklist

  • PR has been tested manually in a VM (either author or reviewer)
  • Jira issue has been made public if possible
  • [RHELC-] or [HMS-] is part of the PR title
  • GitHub label has been added to help with Release notes
  • PR title explains the change from the user's point of view
  • Code and tests are documented properly
  • The commits are squashed to as few commits as possible (without losing data)
  • When merged: Jira issue has been updated to Release Pending if relevant

@SerCantus SerCantus added the kind/tests Improvement or enhancement of the tests label Jun 18, 2024
@SerCantus SerCantus requested a review from danmyway June 18, 2024 08:34
@SerCantus SerCantus self-assigned this Jun 18, 2024
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.51%. Comparing base (bd8b563) to head (76087fb).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1265   +/-   ##
=======================================
  Coverage   96.51%   96.51%           
=======================================
  Files          71       71           
  Lines        5077     5077           
  Branches      883      883           
=======================================
  Hits         4900     4900           
  Misses         98       98           
  Partials       79       79           
Flag Coverage Δ
centos-linux-7 92.01% <ø> (ø)
centos-linux-8 92.88% <ø> (ø)
centos-linux-9 92.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Member

@danmyway danmyway left a comment

Choose a reason for hiding this comment

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

Looks great, just a couple of adjustments requested.
Also, please just switch test-coverage-enhancement label for no-changelog, the enhancement serves for larger introductions, we don't use it for single tests :)

@SerCantus SerCantus added the tests/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`. label Jul 12, 2024
@has-bot
Copy link
Member

has-bot commented Jul 12, 2024

/packit test --labels sanity


Comment generated by an automation.

@SerCantus SerCantus requested a review from danmyway July 31, 2024 14:35
Copy link
Member

@danmyway danmyway left a comment

Choose a reason for hiding this comment

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

Looks good now, great job!
Thanks @SerCantus

@danmyway
Copy link
Member

/packit test --labels tier1

@danmyway danmyway added skip/changelog If it should be excluded from changelog or Release notes. Such as infra, reverted PRs, etc. and removed kind/tests Improvement or enhancement of the tests labels Aug 14, 2024
@danmyway
Copy link
Member

Needs a rebase and a test re-run.
Whenever you're available, please, @SerCantus

@SerCantus SerCantus force-pushed the test-error-after-ponr branch from 7c1c16d to 016262e Compare August 26, 2024 09:57
@SerCantus SerCantus requested a review from a team as a code owner August 26, 2024 09:57
@SerCantus SerCantus requested a review from kokesak August 26, 2024 09:57
@SerCantus
Copy link
Member Author

/packit test --labels tier1

Copy link
Member

@kokesak kokesak left a comment

Choose a reason for hiding this comment

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

Looks promising, the test will fail now due to subman registration error. We should hold this PR open till it is working again

@SerCantus
Copy link
Member Author

/packit test --labels tier1

@SerCantus
Copy link
Member Author

/packit test --labels tier1

@kokesak
Copy link
Member

kokesak commented Sep 6, 2024

Seems like that the conversion works fine - no error happens. I went over the logs, and it seems that the echo command happens quite late. It sadly is executed after the conf patch:

[2024-09-05T15:16:47+0000] TASK - [Convert: Patch package manager configuration file] ****************
[2024-09-05T15:16:47+0000] DEBUG - Calling command 'rpm -Vf /etc/yum.conf'

Executing a command:
echo "proxy=localhost" >> /etc/yum.conf


/etc/yum.conf patched.
CONFIGURE_PKG_MANAGER has succeeded

I suggest doing it much sooner, if you put it just after the
"WARNING - The tool allows rollback of any action until this point." then the error does not happen?

@SerCantus
Copy link
Member Author

Seems like that the conversion works fine - no error happens. I went over the logs, and it seems that the echo command happens quite late. It sadly is executed after the conf patch:

[2024-09-05T15:16:47+0000] TASK - [Convert: Patch package manager configuration file] ****************
[2024-09-05T15:16:47+0000] DEBUG - Calling command 'rpm -Vf /etc/yum.conf'

Executing a command:
echo "proxy=localhost" >> /etc/yum.conf


/etc/yum.conf patched.
CONFIGURE_PKG_MANAGER has succeeded

I suggest doing it much sooner, if you put it just after the "WARNING - The tool allows rollback of any action until this point." then the error does not happen?

The issue with this test is that it only fails on automated executions. When I try to so it manually it always passes so I'm lost. I will try your approach.

@danmyway
Copy link
Member

/packit test --labels tier1

@kokesak kokesak force-pushed the test-error-after-ponr branch from 8af878d to 91c7f0c Compare September 27, 2024 07:07
@kokesak
Copy link
Member

kokesak commented Sep 27, 2024

/packit test --labels tier1

@kokesak kokesak force-pushed the test-error-after-ponr branch from 91c7f0c to a5dd447 Compare September 27, 2024 07:27
@kokesak kokesak requested a review from danmyway September 27, 2024 07:27
@kokesak
Copy link
Member

kokesak commented Sep 27, 2024

/packit test --labels tier1

@kokesak
Copy link
Member

kokesak commented Sep 27, 2024

Tier1 is failing now due to the "/plans/tier1/destructive/modified_grub_file/changed_grub_invalid fail" being obsoloete after recent changes. Once this gets fixed, we can re-run the tests here #1391

@kokesak kokesak force-pushed the test-error-after-ponr branch from a5dd447 to d0eab54 Compare September 30, 2024 06:26
@kokesak
Copy link
Member

kokesak commented Sep 30, 2024

/packit test --labels tier1

@kokesak kokesak force-pushed the test-error-after-ponr branch from d0eab54 to 94cab6a Compare September 30, 2024 09:56
@kokesak
Copy link
Member

kokesak commented Sep 30, 2024

/packit test --labels tier1

@kokesak kokesak force-pushed the test-error-after-ponr branch from 94cab6a to 8bbd470 Compare September 30, 2024 12:37
@kokesak
Copy link
Member

kokesak commented Sep 30, 2024

/packit test --labels tier1

SerCantus and others added 2 commits October 2, 2024 09:40
Apply suggestions from code review

Co-authored-by: Daniel Diblik <[email protected]>

Add comment to new test discover

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Add assert for a shell invoke and indentation to comment

Co-authored-by: Martin "kokesak" Litwora <[email protected]>

Update test_error_after_ponr with new TEST_VARS naming

Co-authored-by: Daniel Diblik <[email protected]>

Update how to trigger the PONR failure in the test

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Change the source of the error - internet connection after PONR
@kokesak kokesak force-pushed the test-error-after-ponr branch from 8bbd470 to 6b1533e Compare October 2, 2024 07:40
@kokesak
Copy link
Member

kokesak commented Oct 2, 2024

/packit test --labels tier1

plans/tier1.fmf Outdated Show resolved Hide resolved
Co-authored-by: Daniel Diblik <[email protected]>
Copy link
Member

@kokesak kokesak left a comment

Choose a reason for hiding this comment

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

LGTM

@kokesak kokesak enabled auto-merge (squash) October 2, 2024 15:33
@kokesak kokesak merged commit 04b7e8a into oamg:main Oct 2, 2024
15 of 18 checks passed
@bocekm bocekm mentioned this pull request Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog If it should be excluded from changelog or Release notes. Such as infra, reverted PRs, etc. tests/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants