Skip to content

Conversation

davi0011
Copy link

@davi0011 davi0011 commented Sep 5, 2025

This adds capability for the -f option to parse Subproject labels from the build.xml for CDash.

This also adds capability for using the labels and overwriting the labesl for Subprojects from test cases on generation of CDash XML.

#15

This adds capability for the -f option to parse Subproject labels
from the build.xml for CDash.

This also adds capability for using the labels and overwriting the
labesl for Subprojects from test cases on generation of CDash XML.
Copy link
Contributor

@mdmosby mdmosby left a comment

Choose a reason for hiding this comment

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

This certainly seems to be a functional approach to generate CDash subproject labels from the test cases, but I am concerned that it crosses the abstraction boundaries between TestCase and reporting mechanisms. Specifically, that it required modification of the TestCase class is concerning because now a TestCase needs to know about some CDash concept ("subproject") which has nothing to do with the collection, organization, or execution of TestCase. I don't think we would allow such changes if an external reporting plugin needed similar information specific to how their data was organized/displayed.

Is there an approach that might better fit the abstraction? What about something like the following at the canary scope or via pluggy hook?

def cdash_subproject_labels(case: TestCase) -> list[str]:
    return case.keywords()

Such an approach would keep CDash reporting concepts out of the TestCase definition. Plugins should be able to specialize/override this function if needed to provide different labels, including for labels that may require information that is not naturally provided by the plugin's TestCase derivative (I don't have a concrete example, but am sure there's a use case out there).

Am I missing something that blocks this approach or makes the current implementation a better approach?

@davi0011
Copy link
Author

davi0011 commented Sep 5, 2025

I share many of the concerns with this approach as well. Tim had proposed using the keywords and adding the product to those since that is what is normally used for the labels however, that led to the issue of the global Subproject tags still being needed. Rather than specifying those on the CLI I thought it might be nicer if a TestCase could declare what Subproject it was a part of though a similar interface to the labels.
I also looked at generating the Subproject labels from the -f option (where you would pass a build.xml) but Spack generates a large amount of files each with their own Subproject tag leading to needing to read each one for the full set. Another approach I looked at was trying to generate them off of the labels but I would likely still want that to be an optional behavior since Subprojects are not the default use case and it causes a large amount of clutter in the Subproject tags.

Given that the TestCases' information is already used for XML generation for CDash it didn't feel like the "worst" option to add an optional interface for the Subproject tags, but I am unsure if a Plugin hook would be preferred. Meanwhile I will clean up all the other comments.

Fixed typos and addressed concerns about wording.
@tjfulle
Copy link
Collaborator

tjfulle commented Sep 7, 2025

I agree with @mdmosby, furthermore, CMake doesn't provide a different API hook for a test to specify its subproject label[s] (if any), but infers the subproject from the test labels. I think we should follow their idiom, but also allow for a plugin hook I like @mdmosby suggested. though the plugin name should start with canary_ to satisfy pluggy norms.

This will generate the Subproject labels off of the keywords
to match the way test labels are naturally generated in CDash
(and Canary).
@davi0011 davi0011 force-pushed the subprojects_from_files_and_test_cases branch from 664ee84 to 2a02f6c Compare September 7, 2025 23:52
@davi0011
Copy link
Author

davi0011 commented Sep 7, 2025

I went ahead and swapped it to use the labels already generated by reading the testcases. This could generate extra labels but it removes the need to modify testcase.py at all. This is effectively the same way CDash would generate the global Subproject labels.

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