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

Improve utils.execute_command, add a Pytest selftest for it, and run it in GHA #1243

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Mar 1, 2024

Pytest is configured according to https://docs.pytest.org/en/7.1.x/explanation/goodpractices.html#tests-as-part-of-application-code

I am using the same poetry command for both the ruff and pytest jobs, so that they can share cache.

Fixes #1241

CI: job/rhods/job/rhods-ci-pr-test/2543/ PASSED w/ known issues (see last comment for CI rerun)

@@ -53,13 +53,12 @@ def read_yaml(filename):
return None


def execute_command(cmd):
def execute_command(cmd, print_stdout=True):
Copy link
Member Author

Choose a reason for hiding this comment

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

this is to have an option to quiet down the output in selftests

@jiridanek jiridanek added needs testing Needs to be tested in Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Mar 1, 2024
Copy link
Contributor

github-actions bot commented Mar 1, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
429 0 0 429 100

@jiridanek
Copy link
Member Author

Run this twice, there was only a test fail in "Verify that the must-gather image provides RHODS logs and info", which is known open issue, https://redhat-internal.slack.com/archives/C03V3J222D7/p1707985201274959?thread_ts=1707974794.266549&cid=C03V3J222D7

CI: job/rhods/job/rhods-ci-pr-test/2544/

@jiridanek jiridanek added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Mar 3, 2024
jstourac
jstourac previously approved these changes Mar 3, 2024
Copy link
Member

@jstourac jstourac left a comment

Choose a reason for hiding this comment

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

With my limited knowledge, LGTM. I put just two optional changes.

It's really funny that all I wanted was to revert 2 lines of code, and here you've come with this PR 😄

Thank you.

Copy link

sonarcloud bot commented Mar 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jiridanek jiridanek requested a review from jstourac March 3, 2024 19:42
@jiridanek jiridanek merged commit 92d12f5 into red-hat-data-services:master Mar 4, 2024
12 checks passed
@jiridanek jiridanek deleted the jd_pytest branch March 4, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Various problems with util.execute_command
3 participants