-
Notifications
You must be signed in to change notification settings - Fork 685
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 a product stability test #10606
Add a product stability test #10606
Conversation
af5c22d
to
72eff38
Compare
I propose to waive the codeclimate findings, because some of the similarity issues are not worth the attention. |
return msg | ||
|
||
|
||
def describe_change(difference, name): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seems to be useful but unfortunately it isn't called anywhere.
tests/test_product_stability.py
Outdated
|
||
import ssg.products | ||
|
||
import tests.test_profile_stability as stability |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of importing the test_profile_stability I would prefer to extract the shared code to a separate file.
tests/test_product_stability.py
Outdated
if not compiled_product: | ||
msg = ("Unexpectedly unable to find compiled product file corresponding" | ||
"to the test file {ref}, although the corresponding product has been built. " | ||
"This indicates that a profile we have tests for is missing." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should be a product, not a profile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually if the file doesn't exist, the product loader will throw an error by itself. This is an extremely unlikely scenario, as the built product directory existence has been checked before. Therefore, I remove the exception handling, as the traceback is good enough for such unlikely cases, and the less code the better.
tests/test_product_stability.py
Outdated
return result | ||
|
||
|
||
def get_references(ref_root): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be a better name get_reference_files
Code Climate has analyzed commit c2d7846 and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 52.5% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have seen the code. Then, I have seen that both affected test cases pass in the "Build, Test on Fedora Latest (Container)" GitHub CI jobs. I have examined manually that when I change add and remove some product properties or profile items the test cases start to fail. For example:
[jcerny@thinkpad build{pr/10606}]$ ctest --verbose -R stable-products
UpdateCTestConfiguration from :/home/jcerny/work/git/scap-security-guide/build/DartConfiguration.tcl
UpdateCTestConfiguration from :/home/jcerny/work/git/scap-security-guide/build/DartConfiguration.tcl
Test project /home/jcerny/work/git/scap-security-guide/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 16
Start 16: stable-products
16: Test command: /usr/bin/env "PYTHONPATH=/home/jcerny/work/git/scap-security-guide" "/usr/bin/python3" "/home/jcerny/work/git/scap-security-guide/tests/test_product_stability.py" "/home/jcerny/work/git/scap-security-guide/build" "/home/jcerny/work/git/scap-security-guide/tests/data/product_stability"
16: Working Directory: /home/jcerny/work/git/scap-security-guide/build/tests
16: Test timeout computed to be: 10000000
16: Following properties got different values in the rhel9 product:
16: - faillock_path from '/var/log/faillock' -> '/var/log/faillock.d'
16: If changes to mentioned products are intentional, copy those compiled files, so they become the new reference:
16: cp '/home/jcerny/work/git/scap-security-guide/build/rhel9/product.yml' '/home/jcerny/work/git/scap-security-guide/tests/data/product_stability/rhel9.yml'
16: If those changes are unwanted, take a look at product properties that likely cause these changes.
1/1 Test #16: stable-products ..................***Failed 0.26 sec
0% tests passed, 1 tests failed out of 1
Label Time Summary:
quick = 0.26 sec*proc (1 test)
Total Test time (real) = 0.26 sec
The following tests FAILED:
16 - stable-products (Failed)
Errors while running CTest
Output from these tests are in: /home/jcerny/work/git/scap-security-guide/build/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
[jcerny@thinkpad build{pr/10606}]$ ctest --verbose -R stable-products
UpdateCTestConfiguration from :/home/jcerny/work/git/scap-security-guide/build/DartConfiguration.tcl
UpdateCTestConfiguration from :/home/jcerny/work/git/scap-security-guide/build/DartConfiguration.tcl
Test project /home/jcerny/work/git/scap-security-guide/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 16
Start 16: stable-products
16: Test command: /usr/bin/env "PYTHONPATH=/home/jcerny/work/git/scap-security-guide" "/usr/bin/python3" "/home/jcerny/work/git/scap-security-guide/tests/test_product_stability.py" "/home/jcerny/work/git/scap-security-guide/build" "/home/jcerny/work/git/scap-security-guide/tests/data/product_stability"
16: Working Directory: /home/jcerny/work/git/scap-security-guide/build/tests
16: Test timeout computed to be: 10000000
16: Following properties were added to the rhel9 product:
16: - my_variable
16: Following properties got different values in the rhel9 product:
16: - dconf_gdm_dir from 'distro.d' -> 'gdm.d'
16: If changes to mentioned products are intentional, copy those compiled files, so they become the new reference:
16: cp '/home/jcerny/work/git/scap-security-guide/build/rhel9/product.yml' '/home/jcerny/work/git/scap-security-guide/tests/data/product_stability/rhel9.yml'
16: If those changes are unwanted, take a look at product properties that likely cause these changes.
1/1 Test #16: stable-products ..................***Failed 0.26 sec
0% tests passed, 1 tests failed out of 1
Label Time Summary:
quick = 0.26 sec*proc (1 test)
Total Test time (real) = 0.27 sec
The following tests FAILED:
16 - stable-products (Failed)
Errors while running CTest
Output from these tests are in: /home/jcerny/work/git/scap-security-guide/build/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
I'm OK with the Code Climate because it is common that argument parsing function in Python code look similar. |
Description:
With the recent introduction of the product class in #10529 and planned introduction of product properties in #10554, a mechanism that can make sure that changes to product properties are under control seems to be needed.
The following test makes sure that if product properties are changed, such changes are confirmed.
It may be that in order to avoid situations s.a. the need to update all test references manually when a new property is introduced, the test script could have this as an option. Automatic update of references would still show in the PR diff, which would preserve the element of confirmation.
Rationale:
Review Hints:
stable-products
tests, or the test script directly.