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

Add --json tests for repo and advisory list commands #1532

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

kontura
Copy link
Contributor

@kontura kontura commented Jul 26, 2024

  • Add a step to compare json outputs
  • Add --json tests for repo and advisory list commands
  • Removes unused dnf4 parts from updateinfo.feature

@pep8speaks
Copy link

pep8speaks commented Jul 26, 2024

Hello @kontura! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-08-01 05:51:05 UTC

The json string values can contain fnmatch patterns.

Adding a new step rather than using "stdout matches line by line"
because json format contains a lot of brackets that would need to be
escaped when the whole line is considered a regex.
It would make the expected output hard to read.

In addition to that the ordering can vary, this step compares
ordered jsons.
@kontura kontura force-pushed the json branch 3 times, most recently from e46cfbb to 15ad5bd Compare July 29, 2024 09:15
@kontura kontura marked this pull request as ready for review July 29, 2024 09:59
@m-blaha m-blaha self-assigned this Jul 31, 2024
"""


Scenario: Repo list without arguments --json --refresh --quiet
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for this scenario? It seems to be essentially the same as the Repo list without arguments --json, and I'm not sure why the --refresh and --quiet options are important. Could you clarify their significance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was meant to test that <REPOSYNC> doesn't get into the stdout.
It was really confusing and wrong.

The --quiet is not needed because its implicitly set by --json and I have changed it to repo info because repo list doesn't need metadata.
I have updated it.

Thank you for catching that.

@m-blaha m-blaha merged commit 589e793 into rpm-software-management:main Aug 1, 2024
5 checks passed
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.

3 participants