[avocado_tests] added default_mapping test #3286
Conversation
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
There was a problem hiding this comment.
Can you rebase to main branch please, which has the fix for the snap build
Also follow the guidelines and add the Signed-off-by line at the bottom, which can easily be done via git commit -s ...
Also add the Closes: line to the issue that it closes and resolves
| def test_default_mapping(self): | ||
| self.assertFileExists('/etc/sos/cleaner/default_mapping') | ||
| self.assertOutputContains('Wrote mapping to') | ||
| with open('default_mapping') as ref: |
There was a problem hiding this comment.
What file you refer to here? When I run this test I get:
(1/8) tests/cleaner_tests/full_report/full_report_run.py:FullCleanTest.test_default_mapping: ERROR: [Errno 2] No such file or directory: 'default_mapping' (2.26 s)
Also I dont understand what you aim to compare here. self.files[0][1] should be /etc/sos/cleaner/default_mapping which will be populated (from scratch) by a mapping of sos report --clean - what content are you trying to compare with?
Maybe it makes sense to check if the file contains (some? all?) from hostname_map / ip_map / ipv6_map / mac_map / keyword_map / username_map strings (which should be in fact keys for nested maps)?
There was a problem hiding this comment.
Yea that was what I was going for but I did not consider that default_mapping would generate machine specific information. Updating commit to change that
dtzhou2
force-pushed
the
dzhou-add-default-map-test
branch
from
16b068f
to
e63eb0d
Compare
| map_count = ref_data.count("map") | ||
| if map_count == 0: | ||
| assert(False) | ||
| assert True |
There was a problem hiding this comment.
Can't we simply:
self.assertNotEqual(ref_data.count("_map"), 0)
?
Also please squash this commit into the other one.
|
Furter, could you please add the
? To have it covered in any |
|
This looks mostly correct - just echoing Pavel's comments above. Additionally please add a DCO line (you can do this with |
dtzhou2
force-pushed
the
dzhou-add-default-map-test
branch
from
e63eb0d
to
f816b8f
Compare
|
/packit retest-failed |
|
dont know whats broken here. Build just says there is a merge conflict |
|
There's a merge conflict with tests/cleaner_tests/basic_function_tests/report_with_mask.py - from 02d52f7 which already added |
[avocado_tests] added default_mapping test Added default mapping test and made it so all cleaner tests have the --no-update option as in issue sosreport#3254 Signed-off-by: Daniel Zhou <dzhou@redhat.com>
dtzhou2
force-pushed
the
dzhou-add-default-map-test
branch
from
f816b8f
to
14995ba
Compare
| self.assertOutputContains('Wrote mapping to') | ||
| with open('/etc/sos/cleaner/default_mapping') as ref: | ||
| ref_data = ref.read() | ||
| map_count = ref_data.count("map") |
There was a problem hiding this comment.
Isn't this line redundant?
|
ACK up to the one redundant line. |
Added default mapping test and made it so all cleaner tests have the --no-update option as in issue #3254
Close #3254
Signed-off-by: Daniel Zhou dzhou@redhat.com
Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines