Skip to content

[tests] Add tests for skip/enable-plugins merge from config and cmdline#4358

Open
jcastill wants to merge 1 commit into
sosreport:mainfrom
jcastill:jcastillo-add-test-4327
Open

[tests] Add tests for skip/enable-plugins merge from config and cmdline#4358
jcastill wants to merge 1 commit into
sosreport:mainfrom
jcastill:jcastillo-add-test-4327

Conversation

@jcastill

@jcastill jcastill commented Jun 4, 2026

Copy link
Copy Markdown
Member

Verify that skip-plugins and enable-plugins are merged (not replaced)
when specified in both sos.conf and command line.

This is a regression test for commit 8fc655a, which fixed the merge
behavior. The new tests verify both effective options
logging in sos.log and actual plugin execution.


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

@jcastill jcastill added the Kind/Testing Related to Testing label Jun 4, 2026
@jcastill

jcastill commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

Some quick notes and questions:

  • I based the *_sos.conf files on the one we have for report_tests/option_tests, but I can clean them up by removing the commented options and leaving just what's needed for the tests.
  • I have my doubts about the test location. I have them live inside option_tests because of the cmdline options, but then I decided to move them to a specific new location inside plugins. Does this location make sense?
  • Any improvements regarding test names are welcome, as always :)
  • Considered adding a test for preset vs config file vs cmdline, to make it more complete. Thoughts on this?

@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/sosreport-sos-4358
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@arif-ali

arif-ali commented Jun 4, 2026

Copy link
Copy Markdown
Member
  • I based the *_sos.conf files on the one we have for report_tests/option_tests, but I can clean them up by removing the commented options and leaving just what's needed for the tests.

I agree, let's clean up the comments, at least that's my opinion

  • I have my doubts about the test location. I have them live inside option_tests because of the cmdline options, but then I decided to move them to a specific new location inside plugins. Does this location make sense?

I don't have an opinion on that, as long as we're testing

  • Any improvements regarding test names are welcome, as always :)

Naming is cool with me

  • Considered adding a test for preset vs config file vs cmdline, to make it more complete. Thoughts on this?

I think that would make sense, then it covers all the options

Verify that skip-plugins and enable-plugins are merged (not replaced)
when specified in both sos.conf and command line. Also verify that
the option only-plugins uses replacement (not merge) approach,
and that preset options take precedence over config file values.

This is a regression test for commit 8fc655a, which fixed
the merge behavior. The new tests verify both effective options
logging in sos.log and actual plugin execution.

Signed-off-by: Jose Castillo <jcastillo@redhat.com>
@jcastill jcastill force-pushed the jcastillo-add-test-4327 branch from 5a33840 to 15d138a Compare June 11, 2026 12:16
@jcastill

Copy link
Copy Markdown
Member Author

I've added some extra tests to cover more use cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Kind/Testing Related to Testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants