Skip to content

Handle skipped tests in "Not run" column - #3743

Open
jjomier wants to merge 9 commits into
Kitware:masterfrom
jjomier:jjomier/handle_skipped_tests
Open

Handle skipped tests in "Not run" column#3743
jjomier wants to merge 9 commits into
Kitware:masterfrom
jjomier:jjomier/handle_skipped_tests

Conversation

@jjomier

@jjomier jjomier commented May 27, 2026

Copy link
Copy Markdown
Contributor

This commit introduces a new notRunSkippedDetailsRegex property to the Project model, allowing users to define regex patterns for not-run test details. A corresponding validation rule is added to ensure the regex is valid. The Build model now includes a method to count not-run tests that do not match the specified patterns, and the GraphQL schema is updated to expose this functionality. Additionally, UI components are modified to display the warning count and adjust test status colors based on the new regex patterns. Tests are also added to verify the new functionality.

image image

@williamjallen williamjallen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm conflicted on whether this belongs in CDash, or whether this should be added as a CTest property. I'm leaning towards the latter because it's possible that some projects will want to dynamically set the list of not run test warnings. The build error filter feature removed in #3650 had roughly the same considerations. Any thoughts on this?

@jjomier

jjomier commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

It could be in addition to having a "Skipped" test property in CTest. However in CDash we would need a way to handle the display of skipped tests. Where you thinking something else?

@williamjallen

Copy link
Copy Markdown
Collaborator

@jjomier Is your goal just to differentiate between disabled tests and tests which were "not run" for a different reason?

We want to minimize the number of configuration options if possible, so I'm hesitant to approve this as-is. After discussing this internally, we think the best approach is to just match the text "Disabled" in the details field without it being user-configurable. Thoughts?

@jjomier

jjomier commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@williamjallen yes the idea is to not pollute the tests that are marked as not run when we look at the dashboard. I like the idea of matching "Disabled".

@jjomier
jjomier force-pushed the jjomier/handle_skipped_tests branch from ef9cd03 to fc65763 Compare August 7, 2026 12:16
@jjomier
jjomier requested a review from williamjallen August 10, 2026 16:41
Comment thread app/Models/Build.php Outdated
Comment thread app/Utils/TestDisplay.php Outdated
Comment thread resources/js/vue/components/shared/FormSection.vue
This commit introduces a new `notRunSkippedDetailsRegex` property to the Project model, allowing users to define regex patterns for not-run test details. A corresponding validation rule is added to ensure the regex is valid. The Build model now includes a method to count not-run tests that do not match the specified patterns, and the GraphQL schema is updated to expose this functionality. Additionally, UI components are modified to display the warning count and adjust test status colors based on the new regex patterns. Tests are also added to verify the new functionality.
This commit removes the `notRunSkippedDetailsRegex` property from the Project model and associated validation rule, simplifying the logic for handling not-run tests. The `notRunTestsWarningCount` method in the Build model is updated to exclude tests with details marked as "Disabled". The GraphQL schema and UI components are adjusted accordingly to reflect these changes. Additionally, tests are updated to ensure correct behavior without the regex pattern handling.
This commit improves the formatting of the `testStatusToColorClass` and `testStatusToTextColorClass` functions in the TestDisplay component. The switch cases have been reformatted for better readability, maintaining the same functionality. No changes to logic or behavior were made.
This commit simplifies the `notRunTestsWarningCount` method in the Build model by using a more efficient query to count not-run tests that do not have details marked as "Disabled". Additionally, the API response in the Index controller is updated to reflect the new method signature. Unused test methods in the TestDisplayTest class have been removed to clean up the codebase.
Replace the App\Utils\TestDisplay helper with a notRunWarning() query scope
on the Test model, so the "Disabled" completion status is interpreted in one
place next to the data it describes. The status color class is derived
directly from the existing Test::DISABLED constant where it is needed, and
the unused text/GraphQL color class helpers are dropped.
Counting every test of every build was only viable for small instances,
since the index page renders many builds at once.  Record the number of
not-run tests worth warning about in a new build.testnotrunwarning column
as test results are submitted, and read that column instead, so the index
page cost no longer scales with the number of tests.
Nothing links to a specific settings section now that the not-run details
are matched without a per-project option, so the anchor prop on FormSection
and the hash navigation it existed for are unused.
The text-color mapping was leftover from the old PHP TestDisplay utility
and is not imported anywhere in the Vue UI.
Master moved the configure/build/test status colors onto BuildTimelineCard.
Use notRunTestsWarningCount there so tests whose details are "Disabled" do
not turn the Test step orange.
@jjomier
jjomier force-pushed the jjomier/handle_skipped_tests branch from 3fda663 to a887248 Compare August 25, 2026 20:03
@jjomier
jjomier requested a review from williamjallen August 25, 2026 20:04
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.

2 participants