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

Compare SUT package versions in investigation, if available #5643

Merged
merged 1 commit into from
May 19, 2024

Conversation

marmarek
Copy link
Contributor

In addition to showing diff of worker_packages.txt, include also a diff
of sut_packages.txt. Creating the file is up to the test, but do add it
to list of files uploaded at the job end.

Document how to use this feature. And while at it, document also
worker_packages.txt and PACKAGES_CMD setting.

Copy link

github-actions bot commented May 16, 2024

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-docs

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

@marmarek
Copy link
Contributor Author

With this, the sut_packages.txt file gets uploaded, but I don't see it listed in "Result files". I'm not sure if worker_packages.txt gets listed (never used this functionality). Is it expected? Or do I need to add it to some list somewhere?

Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.40%. Comparing base (049c328) to head (4d80c34).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5643   +/-   ##
=======================================
  Coverage   98.40%   98.40%           
=======================================
  Files         393      393           
  Lines       38318    38319    +1     
=======================================
+ Hits        37708    37709    +1     
  Misses        610      610           

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

docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
@Martchus
Copy link
Contributor

With this, the sut_packages.txt file gets uploaded, but I don't see it listed in "Result files". I'm not sure if worker_packages.txt gets listed (never used this functionality). Is it expected? Or do I need to add it to some list somewhere?

I would have expected the file to show up there. You have already added it to the relevant list of files and I wouldn't know where to add it additionally.

We also won't seem to actually use worker_packages.txt in production. However, I have just executed sudo -u geekotest touch /var/lib/openqa/testresults/04201/04201194-openqa-Tumbleweed-dev-x86_64-Build\:TW.28542-openqa_from_bootstrap@64bit-2G/worker_packages.txt on one of our production hosts and the empty file now shows up on https://openqa.opensuse.org/tests/4201194#downloads.

@marmarek
Copy link
Contributor Author

I would have expected the file to show up there. You have already added it to the relevant list of files and I wouldn't know where to add it additionally.

I guess I was looking at not patched version, now it does show up.

And BTW, here is how the feature works:
https://openqa.qubes-os.org/tests/99850#investigation

Copy link
Member

@kalikiana kalikiana left a comment

Choose a reason for hiding this comment

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

Note some minor points on the phrasing. Otherwise looks good

docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
docs/WritingTests.asciidoc Outdated Show resolved Hide resolved
@marmarek
Copy link
Contributor Author

Note some minor points on the phrasing.

Thanks, updated.

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.

This looks great! I wasn't considering that sut packages can be used just the same. You found a pretty clean way to do that without needing to have any SUT/OS specific implementations, great!

@marmarek
Copy link
Contributor Author

Looks like codecov status isn't reported (that's why it didn't got merged yet)

@okurz
Copy link
Member

okurz commented May 19, 2024

yeah, codecov isn't always that stable. https://app.circleci.com/pipelines/github/os-autoinst/openQA/13581/workflows/cb0894d1-7d56-4d0c-a8e4-b5afac1b43c1/jobs/127466 shows an error regarding github rate limit although we use a token. Let's rebase with the help of mergify and give the CI pipeline another try

@okurz
Copy link
Member

okurz commented May 19, 2024

@Mergifyio rebase

In addition to showing diff of worker_packages.txt, include also a diff
of sut_packages.txt. Creating the file is up to the test, but do add it
to list of files uploaded at the job end.

Document how to use this feature. And while at it, document also
`worker_packages.txt` and `PACKAGES_CMD` setting.
Copy link
Contributor

mergify bot commented May 19, 2024

rebase

✅ Branch has been successfully rebased

@mergify mergify bot merged commit fcb740d into os-autoinst:master May 19, 2024
42 checks passed
os-autoinst-bot pushed a commit to os-autoinst-bot/openQA that referenced this pull request May 20, 2024
commit fcb740d
Merge: 049c328 4d80c34
Author:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AuthorDate: Sun May 19 11:48:35 2024 +0000
Commit:     GitHub <noreply@github.com>
CommitDate: Sun May 19 11:48:35 2024 +0000

    Merge pull request os-autoinst#5643 from marmarek/sut-packages

    Compare SUT package versions in investigation, if available
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

4 participants