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.

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 Smoke Tests for Some Scripts #9787

Merged
merged 8 commits into from
Nov 10, 2022

Conversation

Mab879
Copy link
Member

@Mab879 Mab879 commented Nov 8, 2022

Description:

  • Adds a basic smoke test for the following scripts
    • utils/compare_ds.py
    • utils/create_srg_export.py
    • utils/srg_diff.py

Rationale:

Improve CI coverage.

@Mab879 Mab879 added enhancement General enhancements to the project. Infrastructure Our content build system labels Nov 8, 2022
@Mab879 Mab879 added this to the 0.1.65 milestone Nov 8, 2022
@github-actions
Copy link

github-actions bot commented Nov 8, 2022

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

The CI fails look legit

COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/utils/create_srg_export.py" --root "${CMAKE_SOURCE_DIR}" --json "${CMAKE_SOURCE_DIR}/build/rule_dirs.json" --control "${CMAKE_SOURCE_DIR}/controls/srg_gpos.yml" --product rhel9 --out-format xlsx --output "${CMAKE_BINARY_DIR}/cac_stig_output.xlsx" --build-config-yaml "${CMAKE_BINARY_DIR}/build_config.yml"
)
set_tests_properties("srg-export-rhel9" PROPERTIES LABELS quick)
set_tests_properties("srg-export-rhel9" PROPERTIES DEPENDS "test-rule-dir-json")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like the DEPENDS on other tests, because I can't run the test standalone using the -R option. I was looking into fixtures https://cmake.org/cmake/help/latest/prop_test/FIXTURES_REQUIRED.html but they are new in CMake 3.7. So I would propose to create the rule_dir.json as a part of this test.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will need to do some research but my initial reaction to that is that we might not be be able to use fixtures as RHEL 7 has CMake 2.7.

Copy link
Member Author

Choose a reason for hiding this comment

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

It appears we can use fixures even with the older CMake that RHEL 7 uses. I left the DEPENDS so that we are backward compatible in the full runs.

@jan-cerny jan-cerny self-assigned this Nov 9, 2022
This allow for running one test with `ctest -R` with CMake 3.7+.
Make sure that only Python 3 is used and all python deps are
installed.
@codeclimate
Copy link

codeclimate bot commented Nov 9, 2022

Code Climate has analyzed commit 9abae2e and detected 0 issues on this pull request.

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 46.8% (0.0% change).

View more on Code Climate.

@Mab879
Copy link
Member Author

Mab879 commented Nov 9, 2022

/packit retest-failed

Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

This is an excellent improvement!

@jan-cerny jan-cerny merged commit 6912c51 into ComplianceAsCode:master Nov 10, 2022
@Mab879 Mab879 deleted the smoke_test_srg_diff branch November 11, 2022 22:00
@matejak
Copy link
Member

matejak commented Nov 28, 2022

Thanks, I like those new smoke tests.
At the same time, adding thousands of lines of data to conduct a smoke test seems to be quite a lot, wouldn't it be possible to simplify the test data somehow in another PR?

@Mab879
Copy link
Member Author

Mab879 commented Nov 29, 2022

It should be possible; we would need to write XML files that follow how DISA does it.

@matejak
Copy link
Member

matejak commented Nov 30, 2022

I was thinking in the opposite direction - take the current test content and throw about 99% of it away, so only matching skeletons remain. Skeleton in, skeleton out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancements to the project. Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants