-
Notifications
You must be signed in to change notification settings - Fork 9
Workspace flow + TestSpec generation #67
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
base: main
Are you sure you want to change the base?
Conversation
…cation should now work
… checking). begin to look for `start` argument of pathspec
…ate it. fixed up garbage collecting
mdmosby
left a comment
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.
Minor things that can wait
|
|
||
| @canary.hookimpl(wrapper=True) | ||
| def canary_cdash_artifacts(case: canary.TestCase) -> Generator[None, None, list[dict[str, str]]]: | ||
| """Default implementation: return the test case's keywords""" |
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.
| """Default implementation: return the test case's keywords""" | |
| """Default implementation: return the test case's artifact specifications""" |
Should canary.TestCase use this Schema to validate the artifacts specification?
| rparameters: dict[str, int] | ||
| assets: list["Asset"] | ||
| baseline: list[dict] | ||
| artifacts: list[dict[str, str]] |
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.
Should we validate the artifacts Schema as part of post_init?
| artifacts = list(case.spec.artifacts) or [] | ||
| result = yield | ||
| artifacts.extend([_ for _ in result if _]) | ||
| for i, artifact in enumerate(artifacts): | ||
| artifacts[i] = schema.validate(artifact) | ||
| return artifacts |
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.
What is the expected behavior if artifacts include elements with duplicate file values and conflicting when values?
`Path.hardlink_to` requires that `src` and `dst` are on the same **filesystem** which is a more-stringent requirement than we've previously had. Furthermore, `hardlink_to` does not follow copy symantics in that modifying the hardlinked file _also modifies the source_! Which may be unexpected behavior when one specifies `copy`.
1. Generator plugins must now implement canary_collect_file_patterns that returns a list of glob patterns that the generator recognizes 2. During discovery, we walk the tree and collect all files matching patterns from above 3. After collection, we call another new plugin canary_collect_filter_files. This plugin can filter out files from the list collected. This is useful for the ctest plugin to remove all but the first CTestTestfile in a tree 4. After filtering, create test case generators in parallel. Additionally, store all generator meta data in a single file. With the above changes, time to collect generators for a large test suite went from 25-30 seconds to less than 1 second.
This PR supersedes #66 and includes all its details. Additionally, it includes a major change to the generation of test cases as described in spec-case.md