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.

Sign up for GitHub

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

Jump to bottom

Make test suite tests more accessible #6675

Merged
merged 4 commits into from Mar 15, 2021
Merged

Make test suite tests more accessible #6675

merged 4 commits into from Mar 15, 2021

Conversation

matejak
Copy link
Member

@matejak matejak commented Mar 10, 2021

The PR introduces fixes to the test suite as well as to the container testing wrapper:

Fixed machine-only handling in the test suite.

  • Fixed query for Bash fixes.
  • Support for empty remediations.

Refreshed the container test wrapper.

  • Added support for --dontclean
  • Simplified the end-result command-line composition.
  • Enabled the --dry-run option that prints the full test suite command-line.

@matejak matejak added this to the 0.1.56 milestone Mar 10, 2021
@openscap-ci
Copy link
Collaborator

openscap-ci commented Mar 10, 2021 •
edited

Changes identified:
Others:
 Changes in Python files.

Show details

Others:
 Python abstract syntax tree change found in tests/ssg_test_suite/xml_operations.py.

Recommended tests to execute:
 (cd build && cmake ../ && ctest -j4)

@JAORMX
Copy link
Contributor

JAORMX commented Mar 10, 2021

/test e2e-aws-rhcos4-moderate

jan-cerny
jan-cerny requested changes Mar 15, 2021
View reviewed changes
tests/test_rule_in_container.sh Outdated
printf '\t%s\n' "<rule>: The short rule ID. Wildcards are supported."
printf '\t%s\n' "-n, --name: Name of the test image (default: 'ssg_test_suite')"
printf '\t%s\n' "-s, --scenarios: Regex to reduce selection of tested scenarios (default: '.*')"
printf '\t%s\n' "-d, --datastream: Path to the datastream to use in tests. Autodetected by default. (no default)"
printf '\t%s\n' "-r, --remediate-using: What to remediate with. Can be one of: 'oscap', 'bash' and 'ansible' (default: 'oscap')"
printf '\t%s\n' "--dontclean, --no-dontclean: Dont remove HTML reports from the log directory. (off by default)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤪

Totally unsure what "off by default" means here. What is "no dontclean"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Off refers to the option, i.e. it means that --dontclean is not considered by default. I agree that this double negative (don't + no) is unfortunate.

tests/test_rule_in_container.sh Outdated
printf '\t%s\n' "<rule>: The short rule ID. Wildcards are supported."
printf '\t%s\n' "-n, --name: Name of the test image (default: 'ssg_test_suite')"
printf '\t%s\n' "-s, --scenarios: Regex to reduce selection of tested scenarios (default: '.*')"
printf '\t%s\n' "-d, --datastream: Path to the datastream to use in tests. Autodetected by default. (no default)"
printf '\t%s\n' "-r, --remediate-using: What to remediate with. Can be one of: 'oscap', 'bash' and 'ansible' (default: 'oscap')"
printf '\t%s\n' "--dontclean, --no-dontclean: Dont remove HTML reports from the log directory. (off by default)"
printf '\t%s\n' "--dry-run, --no-dry-run: Just print the test suite command-line. (off by default)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is similar but not so terrible. However, I really don't understand the purpose of --no-dry-run. If I don't want to run a dry run I simply don't type --dry-run.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a theoretical possibility that you have command options stored in an array, but you want to override any prior --dry-run that may or may not be there, and you would do that by adding --no-dry-run to the array. But again, I agree that it is unfortunate.

@jan-cerny jan-cerny self-assigned this Mar 15, 2021
@jan-cerny
Copy link
Collaborator

@matejak I have a new problem: the command generated by the dry run option doesn't work for me.

[jcerny@localhost scap-security-guide{pr/6675}]$ tests/test_rule_in_container.sh --dry-run ensure_logrotate_activated
python3 /home/jcerny/work/git/scap-security-guide/tests/test_suite.py rule --remove-machine-only --scenario .* --remediate-using oscap --add-platform fedora --container ssg_test_suite -- ensure_logrotate_activated
[jcerny@localhost scap-security-guide{pr/6675}]$ python3 /home/jcerny/work/git/scap-security-guide/tests/test_suite.py rule --remove-machine-only --scenario .* --remediate-using oscap --add-platform fedora --container ssg_test_suite -- ensure_logrotate_activated
usage: test_suite.py [-h] {profile,rule,combined} ...
test_suite.py: error: unrecognized arguments: -- ensure_logrotate_activated

@jan-cerny
Copy link
Collaborator

I think it's because the .* needs quotes

@matejak
Fixed machine-only handling in the test suite.
006cf02 
- Fixed query for Bash fixes.
- Support for empty remediations.
@matejak
Refreshed the container test wrapper.
4416a24 
- Added support for --dontclean
- Simplified the end-result command-line composition.
- Enabled the --dry-run option
@matejak
Updated the readme.
fe9e92e 
- Cleaned up an ancient merge conflict.
- Added documentation about the package metadata.
@matejak
Improved the wrapper script.
393f935 
- Removed -no-... command-line options - they don't make sense in this context.
- Fixed the --scenario option - make the default value to the string, and make sure that the value is quoted in the dry run mode.
@jan-cerny jan-cerny merged commit 2edf7c9 into ComplianceAsCode:master Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan-cerny jan-cerny jan-cerny requested changes

Assignees

@jan-cerny jan-cerny

Labels
None yet
Projects
None yet
Milestone
0.1.56
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@matejak @openscap-ci @JAORMX @jan-cerny