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

Profile resolution spec and top-level unit tests #1479

Draft
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

wendellpiez
Copy link
Contributor

Committer Notes

Much work in progress in developing Profile Resolution unit tests and docs.

Addresses #1087.

All Submissions:

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.

david-waltermire and others added 13 commits May 17, 2022 08:11
* Many fixes to the constraints in the OSCAL metaschemas to repair broken Metapaths.
* fixing defects in metaschema constraints
* Updating to latest Metaschema toolchain. Removed use of the "require" constraint.
* updating readme with current links
…ts (usnistgov#1101)

* Group scenarios and add edge case scenario
* Group the scenarios that test o:glob-as-regex.
* Add scenario for edge case where input is empty.
* For selection, augment XSpec and update XSLT
* Add XSpec tests for oscal-profile-resolve-select.xsl, mostly at the level of templates and functions.
* Minor enhancements in oscal-profile-resolve-select.xsl:
* Add support for with-parent-controls
* Generate fatal error if resource cannot be fetched
* Provide focused error message if resource has no suitable rlink
* Handle missing matching pattern
* Fix indentation
* Fix scenario that intentionally omits pattern
* Rename o:resource-or-warning as o:resource-or-error
* Attributes, not elements, for "from" and "to"
* Update expected value for 4a3cadf changes in catalog
* The abc-full_catalog.xml file has changed, so the "Loose parameters" test scenario needs adjustment.
* Reduce redundancy in expected metadata
* metadata is copied verbatim, so no need to check details in multiple test scenarios for the match=profile template
* Update verbiage to cross-reference with logged discussion pages
For consistency with select.xspec, use XSpec variable instead of entity
for the path to the profile resolution examples.
random-util.xsl provides r:make-uuid and r:make-uuid-sequence for creating one
or more random UUIDs. The r:make-random-string-sequence function supports
more flexible random patterns. Implementations here use the XPath
function, random-number-generator.

random-util.xspec provides template- and function-level tests for
code in random-util.xsl.
Implementing an idea from @wendellpiez: Instead of iterating over a
numeric sequence that represents the position of each character in a
long string, iterate over the sequence of characters.

This change is in the implementation only, not in the function output.
I used ad hoc modifications of the label="seq-length=10000" scenario
in the XSpec test to check that the original and modified functions
produce the same UUID sequence.
…istgov#1175)

- Metadata tests and way of determining top UUID
- Remove global parameter assign-uuid (note backward incompatibility - is it OK?)
- Instead, support global parameters uuid-method and top-uuid, in uuid-method-choice.xsl
- Support global parameter hide-source-profile-uri
- Stub of opr:oscal-version function
- Stub of message handler template, using xsl:message for now
- Add XSpec tests
- Add table of parameters and clarify testing folder content
- Change "home" from global param to global variable; does not need to be set from outside
XSLT
- Move some code from oscal-profile-resolve-select.xsl to new file, select-or-custom-merge.xsl, and share it between Selection phase and Merge phase.
- Add error checking for multiple structuring directives.
- Finish the incomplete support for `insert-controls/@order`.
- Fix bug in support for `@with-parent-controls` in Selection phase.

XSpec
- In select.xspec, add test for order of controls matching imported document, not profile document. Also, add test for `@with-parent-controls` bug fix.
- Create merge-combine.xspec for testing combine-elements and mode="o:combine-elements" templates.
- Organize tests among merge.xspec, merge-combine.xspec, merge-as-is.xspec, and merge-custom.xspec.
- Integrate back-matter testing into template-level tests in merge.xspec and merge-as-is.xspec, and delete merge-back-matter.xspec.
* Updates to tests including whitespace scrub test
* Adding oXygen project
* Integration work on specs and unit tests / XSLT and Schematron
* Profile resolution testing infrastructure, plus some tests
* Some test files with expected results
* Updated and improved XSpec generation and tests - now driven directly from SpecML source
* Adjusted gitignore; extended spec with more bindings; illustrator XSLT
* Added XSLT producing Markdown summary of requirements w/ examples; updates
* Added a small utility for converting JSON "prop" fields to "props" fields (emended 1.0.2 syntax)
* Updated to Profile Resolution spec (with example files now tagged) and readme.md
* Updated profile resolution (spec) readme with clarification regarding unit testing
@aj-stein-nist
Copy link
Contributor

Hi @wendellpiez, let's talk about this PR when we catch up later this afternoon if we can.

@iMichaela
Copy link
Contributor

@wendellpiez - This old PR is still marked as work in progress but it has already too many conflicts. Can you please rebase and resubmit it?

@wendellpiez
Copy link
Contributor Author

I can look, but I think there is a reasonable chance that this work has all been merged.

At least, that is what I hope - since I'm somewhat surprised to see the Issue as I thought this work had been merged long ago. But maybe the PR was not closed when the work was merged into another PR for merging.

A separate problem is how we assess the correctness or even adequacy of the work being committed. (I can't say why the work was not merged before, but I suspect this has to do with it.)

@iMichaela
Copy link
Contributor

@wendellpiez - we can close the PR if the work was already merged by other PRs, but this PR was opened in Sept 2022, and was never pulled for review because it is marked as work in progress. I appreciate you taking your time to provide some clarity here.

@wendellpiez
Copy link
Contributor Author

As the comment thread shows, we were looking at this as late as last Oct 2023, when much work was merged from various branches including (or at least I believed at the time) this one. But rather than relying on my partial knowledge and faulty memory, @aj-stein-nist and I intend to look at the actual diffs.

@wendellpiez
Copy link
Contributor Author

For a few days now I have been looking at this one, and coming to the following conclusions

  • There is much fine work here (and not only by me), still to be validated by team and merged in,

However:

  • It includes a 'correction' (model repair) in the profile metaschema that requires broader awareness if not discussion - or to be held back for its own Issue. (It is a no-op 'tidying' that appears to be tightening until examined more closely. Even as a no-op it is a change and will have to be managed as such.)
  • Work in this PR on the Profile Resolution Specification overlaps significantly with work supporting test infrastructure in recent PRs into OSCAL
  • In one PR it lumps together, with other stuff, two separate work items:
    • Conformance testing for profile resolution - for any profile resolver e.g. oscal-xslt or oscal-java - although wired up for XSLT (XSpec)
    • Application functional testing (XSpec) for the XSLT implementation (still in this repository)

Even if we roll back and carve out a separate Issue for the first of these, the second and third have more profound implications.

Application testing for the XSLT could, with the XSLT itself, be carved out of the repository and moved with the application to https://github.com/usnistgov/oscal-xslt.

But the conformance testing suite, with example catalogs and profiles, tied to the Specification, belongs to OSCAL, not to any application -- and raises its own questions.

Even were I able to resolve the conflicts and finalize/ready the PR, until both these sets of tests can be run and validated by other team members, there is no point.

One way forward could be to merge this work into a feature branch for the present. Or maybe two of them - one for conformance testing and work on the spec, and one for just the XSLT implementation (whether stabilizing or carving out).

Splitting the work up could make merging into develop much cleaner and easier.

Comments? (Especially from the more git-knowledgeable than myself.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Profile Resolution Issues relating to profile resolution.
Projects
Status: Under Review
Development

Successfully merging this pull request may close these issues.

7 participants