[tests] Scrub tests #3798

jjansky1 · 2024-10-03T14:01:03Z

Adding stageone and stagetwo tests for [system] plugin.

Also adding tag scrub for only testing scrub of sensitive data.

And updating README with how to call scrub and stagetwo tests.

Related: #3788
Related: #3789
Resolves: #3798

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 protected]?
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?

packit-as-a-service · 2024-10-03T14:03:59Z

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

sudo yum install -y dnf-plugins-core on RHEL 8
sudo dnf install -y dnf-plugins-core on Fedora
dnf copr enable packit/sosreport-sos-3798
And now you can install the packages.

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

Adding stageone and stagetwo tests for [system] plugin. Also adding tag scrub for only testing scrub of sensitive data. And updating README with how to call scrub and stagetwo tests. Related: sosreport#3788 Related: sosreport#3789 Resolves: sosreport#3798 Signed-off-by: Jan Jansky <[email protected]>

tests/test_data/system_test_data

Adding stageone and stagetwo tests for [system] plugin. Also adding tag scrub for only testing scrub of sensitive data. And updating README with how to call scrub and stagetwo tests. Related: sosreport#3788 Related: sosreport#3789 Resolves: sosreport#3798 Signed-off-by: Jan Jansky <[email protected]>

jcastill

From my limited knowledge of avocado tests, LGTM, just need to address Pavel's note about the password string

tests/report_tests/plugin_tests/systemd.py

tests/report_tests/plugin_tests/system.py

Adding stageone and stagetwo tests for [system] plugin. Also adding tag scrub for only testing scrub of sensitive data. And updating README with how to call scrub and stagetwo tests. Related: sosreport#3788 Related: sosreport#3789 Resolves: sosreport#3798 Signed-off-by: Jan Jansky <[email protected]>

pmoravec · 2024-10-07T10:02:59Z

tests/report_tests/plugin_tests/systemd.py

+        self.assertFileGlobInArchive("/etc/systemd")
+        self.assertFileGlobInArchive("/lib/systemd/system")
+        self.assertFileGlobInArchive("/lib/systemd/user")
+        self.assertFileGlobInArchive("/etc/vconsole.conf")


The Glob test is redundantly generic for a direct filename without a glob, but let it be..

Adding stageone and stagetwo tests for [system] plugin. Also adding tag scrub for only testing scrub of sensitive data. And updating README with how to call scrub and stagetwo tests. Related: sosreport#3788 Related: sosreport#3789 Resolves: sosreport#3798 Signed-off-by: Jan Jansky <[email protected]>

tests/report_tests/plugin_tests/system.py

Adding stageone and stagetwo tests for [system] plugin. Also adding tag scrub for only testing scrub of sensitive data. And updating README with how to call scrub and stagetwo tests. Related: sosreport#3788 Related: sosreport#3789 Resolves: sosreport#3798 Signed-off-by: Jan Jansky <[email protected]>

TurboTurtle

Sorry this one sat for so long without another review. Notes below.

TurboTurtle · 2024-11-03T19:44:14Z

tests/report_tests/plugin_tests/system.py

+    files = [
+        ('../../../tests/test_data/system_test_data', '/etc/sysconfig/proxy'),
+        ('../../../tests/test_data/system_test_data', '/etc/sysconfig/proxy1'),
+    ]


The mocked file functionality assumes the files are co-located with the test pyfile because the general design is that each test (which has associated mocked data with it) will be in its own subdir.

Rather than maintaining relative paths like this, place the test file in a subdir under tests/report_tests/plugin_tests and put the test case and the mocked data there.

TurboTurtle · 2024-11-03T19:44:22Z

tests/report_tests/plugin_tests/systemd.py

+    files = [
+        ('../../../tests/test_data/system_test_data', '/etc/systemd/system'),
+        ('../../../tests/test_data/system_test_data', '/lib/systemd/system'),
+        ('../../../tests/test_data/system_test_data', '/run/systemd/system'),
+    ]


TurboTurtle · 2024-11-03T19:44:56Z

tests/report_tests/plugin_tests/systemd.py

+    def test_systemd_files_collected(self):
+        for file in self.files:
+            self.assertFileGlobInArchive(file[1])


I'm not sure why we're testing the collection of files we're mocking?

TurboTurtle · 2024-11-03T19:46:15Z

tests/report_tests/plugin_tests/systemd.py

+        self.assertFileGlobInArchive("journalctl_--list-boots")
+        self.assertFileGlobInArchive("ls_-alZR_.lib.systemd")
+        self.assertFileGlobInArchive("resolvectl_statistics")
+        self.assertFileGlobInArchive("resolvectl_status")
+        self.assertFileGlobInArchive("systemctl_list-dependencies")
+        self.assertFileGlobInArchive("systemctl_list-jobs")
+        self.assertFileGlobInArchive("systemctl_list-machines")
+        self.assertFileGlobInArchive("systemctl_list-timers_--all")
+        self.assertFileGlobInArchive("systemctl_list-unit-files")
+        self.assertFileGlobInArchive("systemctl_list-units")
+        self.assertFileGlobInArchive("systemctl_list-units_--all")
+        self.assertFileGlobInArchive("systemctl_list-units_--failed")
+        self.assertFileGlobInArchive("systemctl_show_--all")
+        self.assertFileGlobInArchive("systemctl_show-environment")
+        self.assertFileGlobInArchive("systemctl_show_service_--all")
+        self.assertFileGlobInArchive("systemctl_status_--all")
+        self.assertFileGlobInArchive("systemd-analyze")
+        self.assertFileGlobInArchive("systemd-analyze_blame")
+        self.assertFileGlobInArchive("systemd-analyze_dump")
+        self.assertFileGlobInArchive("systemd-analyze_plot.svg")
+        self.assertFileGlobInArchive("systemd-delta")
+        self.assertFileGlobInArchive("systemd-inhibit_--list")
+        self.assertFileGlobInArchive("timedatectl")


Should be using assertFileCollected for specific file names.

TurboTurtle · 2024-11-03T19:47:07Z

tests/report_tests/plugin_tests/system.py

+        self.assertFileGlobInArchive("/proc/sys")
+        self.assertFileGlobInArchive("/etc/default")
+        self.assertFileGlobInArchive("/etc/environment")


Same. Use assertFileCollected for specific paths, only use assertFileGlobInArchive if there are actual globs being checked.

TurboTurtle · 2024-11-03T19:47:35Z

tests/report_tests/plugin_tests/system.py

+    def test_system_files_collected(self):
+        for file in self.files:
+            self.assertFileGlobInArchive(file[1])


Again, not sure why we're testing for a collection that we're mocking to begin with.

arif-ali · 2024-11-04T08:22:20Z

Just thinking out loud here, we're not using the tag scrub anywhere. Is there a need for a new tag when we're already doing stageone and stagetwo tests and these are being encompassed by them?

jjansky1 requested review from jcastill and pmoravec October 3, 2024 14:01

jjansky1 force-pushed the main branch from 6c06ff9 to f6be6e9 Compare October 3, 2024 14:10

pmoravec reviewed Oct 3, 2024

View reviewed changes

tests/test_data/system_test_data Outdated Show resolved Hide resolved

jjansky1 force-pushed the main branch from f6be6e9 to 15ac396 Compare October 3, 2024 14:20

jcastill approved these changes Oct 3, 2024

View reviewed changes

pmoravec reviewed Oct 3, 2024

View reviewed changes

tests/report_tests/plugin_tests/systemd.py Outdated Show resolved Hide resolved

pmoravec reviewed Oct 3, 2024

View reviewed changes

tests/report_tests/plugin_tests/system.py Show resolved Hide resolved

jjansky1 force-pushed the main branch from 15ac396 to 4519dc7 Compare October 7, 2024 08:18

jjansky1 force-pushed the main branch from 4519dc7 to a710820 Compare October 7, 2024 09:50

jjansky1 force-pushed the main branch from a710820 to 981164a Compare October 7, 2024 09:55

pmoravec reviewed Oct 7, 2024

View reviewed changes

pmoravec approved these changes Oct 7, 2024

View reviewed changes

jjansky1 force-pushed the main branch from 981164a to 300ed50 Compare October 7, 2024 13:25

jjansky1 force-pushed the main branch from 300ed50 to 0107a9c Compare October 7, 2024 13:29

jcastill reviewed Oct 7, 2024

View reviewed changes

tests/report_tests/plugin_tests/system.py Show resolved Hide resolved

jjansky1 force-pushed the main branch from 0107a9c to 29db47b Compare October 8, 2024 07:23

TurboTurtle reviewed Nov 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests] Scrub tests #3798

[tests] Scrub tests #3798

jjansky1 commented Oct 3, 2024

packit-as-a-service bot commented Oct 3, 2024

jcastill left a comment

pmoravec Oct 7, 2024

TurboTurtle left a comment

TurboTurtle Nov 3, 2024

TurboTurtle Nov 3, 2024

TurboTurtle Nov 3, 2024

TurboTurtle Nov 3, 2024

TurboTurtle Nov 3, 2024

TurboTurtle Nov 3, 2024

arif-ali commented Nov 4, 2024

[tests] Scrub tests #3798

Are you sure you want to change the base?

[tests] Scrub tests #3798

Conversation

jjansky1 commented Oct 3, 2024

packit-as-a-service bot commented Oct 3, 2024

jcastill left a comment

Choose a reason for hiding this comment

pmoravec Oct 7, 2024

Choose a reason for hiding this comment

TurboTurtle left a comment

Choose a reason for hiding this comment

TurboTurtle Nov 3, 2024

Choose a reason for hiding this comment

TurboTurtle Nov 3, 2024

Choose a reason for hiding this comment

TurboTurtle Nov 3, 2024

Choose a reason for hiding this comment

TurboTurtle Nov 3, 2024

Choose a reason for hiding this comment

TurboTurtle Nov 3, 2024

Choose a reason for hiding this comment

TurboTurtle Nov 3, 2024

Choose a reason for hiding this comment

arif-ali commented Nov 4, 2024