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

Testing Toolkit and Build Automation #355

Closed
wants to merge 1 commit into from
Closed

Testing Toolkit and Build Automation #355

wants to merge 1 commit into from

Conversation

howieavp76
Copy link

@howieavp76 howieavp76 commented May 15, 2019

Testing Toolkit and Build Automation

Committer Notes

Initial test suite for validating OSCAL content. Includes automation scripts for conversions, XML validation, JSON validation, timestamp checks, and round trip conversions checks to improve the quality of OSCAL content. Related to issues #321 and #342. In addition, this change allows automated builds of JSON from the XML master data. This is necessary to validate conversion scripts and to ensure proper timestamp checking.

All Submissions:

  • [ x] Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you squashed any non-relevant commits and commit messages? [instructions]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them? See above
  • Have you written new tests for your core changes, as applicable? Tests provided
  • Have you included examples of how to use your new feature(s)? See ReadMe

Killed old fork - cloned from master - recreated testing in the new repository
Copy link
Contributor

@anweiss anweiss left a comment

Choose a reason for hiding this comment

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

LGTM. @howieavp76 I recommend updating the PR title and description to better reflect the fact that you're re-adding the JSON examples, since they're integral to the testing functionality you're merging. Also, nit ... you can remove the erroneous space in [ x] in the checkboxes in the description so they are properly rendered.

@howieavp76 howieavp76 changed the title Testing Changes Testing Toolkit and Build Automation May 15, 2019
@howieavp76
Copy link
Author

@anweiss - updated per your request.

david-waltermire added a commit to david-waltermire/OSCAL that referenced this pull request May 17, 2019
@david-waltermire
Copy link
Contributor

@howieavp76 I merged this PR into PR #358. I removed the scripts from your PR as most of this is handled by the current scripts, which use config files instead of hard coded entries. I am going to close this PR.

@david-waltermire
Copy link
Contributor

@howieavp76 Here is some feedback on the scripts.

  • The specific files to validate/transform are hard coded in the scripts and in the environment using testingConfig.sh. To add a new file to be processed you would need to add an entry to testingConfig.sh and to the respective script. As new files are added, this approach will create a maintenance headache. PR Refactoring CI/CD Pipeline and Repository Directory Structure #358 uses config files to drive what files are validated and transformed, which is a much more maintainable approach.
  • The colorization is nice, but not explicitly needed. I can get the same validation functionality from a CLI for JSON and XML validation without the colors. Not sure the added complexity of having a Python script for validation is needed.
  • The jsonSchemaValidation.py only reports a single error. I added code to loop over and report all errors in PR Refactoring CI/CD Pipeline and Repository Directory Structure #358.
  • The xmlValidation.py writes log files using static names. I don't think log files are needed. This behavior is different from the jsonSchemaValidator.py, and does not work well for validating a large number of files, such as what will occur in the CI/CD pipeline. The naming scheme is also different here as compared to jsonSchemaValidation.py.
  • I don't understand the need for the timestamp compare provided by timeStampValidation.py. If all artifacts are being regenerated by the CI/CD, we know that the latest files will always be there. This isn't mention in CI/CD Improvements to Incorporate Testing and Automation Checks #342 either. Not sure what the genesis of this was.
  • I plan to look into using the xmlComparison.py for round trip tests.

@david-waltermire
Copy link
Contributor

@howieavp76 I also noticed that jsonschema v3 is needed to support JSON schema draft-07. When I tried to install the latest version using pip install 'jsonschema>=3.0.1', I found that docker-compose 1.24.0 has requirement on jsonschema<3,>=2.5.1. This makes use of v3 incompatible with docker-compose. This is also important for #358.

I think it might be best to stick with ajv-cli for JSON validation, which works well.

@anweiss
Copy link
Contributor

anweiss commented May 17, 2019

@david-waltermire-nist that sounds like a local package issue whereby when you use pip to specify a specific version, it will override any previously installed versions and you also happen to have the docker-compose tooling installed via pip. You can avoid this issue by installing the compiled docker-compose binary. See https://docs.docker.com/compose/install/

@howieavp76
Copy link
Author

@david-waltermire-nist @anweiss I will switch the JSON checks to AJV as recommended. The jsonschema tool was underwhelming anyway and this will make life simpler. I will change out all of those checks to match.

david-waltermire added a commit to david-waltermire/OSCAL that referenced this pull request May 17, 2019
david-waltermire added a commit that referenced this pull request May 17, 2019
…358)

* refactored directory structure to match the structure defined in issue #331.
* removed travis config
* removed generated documentation from the repository and added a script to generate them
* initial commit of site build
* Updated readmes to fix broken links and cleaned up unused files
* Added a CircleCI status badge
* merged in python scripts from #355. Enhanced error reporting for JSON validation script.
* Updated out of date JSON files
aj-stein-nist referenced this pull request in aj-stein-nist/OSCAL-forked Jan 25, 2023
…358)

* refactored directory structure to match the structure defined in issue #331.
* removed travis config
* removed generated documentation from the repository and added a script to generate them
* initial commit of site build
* Updated readmes to fix broken links and cleaned up unused files
* Added a CircleCI status badge
* merged in python scripts from #355. Enhanced error reporting for JSON validation script.
* Updated out of date JSON files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants