Skip to content

Make LongVectors tests run as part of standard test passes#7873

Merged
damyanp merged 2 commits intomicrosoft:mainfrom
damyanp:lvector
Nov 11, 2025
Merged

Make LongVectors tests run as part of standard test passes#7873
damyanp merged 2 commits intomicrosoft:mainfrom
damyanp:lvector

Conversation

@damyanp
Copy link
Member

@damyanp damyanp commented Nov 5, 2025

Currently, the various test runners (lit, hcttest.cmd) only run execution tests where the Priority metadata is <2.

Previously, the LongVectors tests didn't set a value for priority, and this resulted in them never being selected. This change adds the missing priority metadata - and then fixes up the tests so they don't immediately fail if the runtime/device doesn't support SM 6.9!

Currently, the various test runners (lit, hcttest.cmd) only run execution tests where the Priority metadata is <2.

Previously, the LongVectors tests didn't set a value for priority, and this resulted in them never being selected.  This change adds the missing priority metadata - and then fixes up the tests so they don't immediately fail if the runtime/device doesn't support SM 6.9!
Co-authored-by: Alex Sepkowski <alexsepkowski@gmail.com>
Copy link
Contributor

@alsepkow alsepkow left a comment

Choose a reason for hiding this comment

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

lgtm

TEST_CLASS_PROPERTY(
"Kits.Specification",
"Device.Graphics.D3D12.DXILCore.ShaderModel69.CoreRequirement")
TEST_METHOD_PROPERTY(L"Priority", L"0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, actually wondering if this is right? I tested it seems to work as I would expect. The individual methods all seem to inherit it. But seeing a TEST_METHOD_PROPERTY inside of the TEST_CLASS definition seems weird.

Did you sanity check that? Wondering if this behavior could change in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

I take that back. It shouldn't compile if it isnt expected to work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it looks weird, but this is how it's done in ExecutionTest.cpp. I'm not sure you'd expect a compilation error due to the way TAEF is implemented, but a runtime error maybe. Practically, it seems to apply the same property to all the methods in the class.

@damyanp
Copy link
Member Author

damyanp commented Nov 10, 2025

/azp run

(Double checking nothing else changed recently!)

@azure-pipelines
Copy link

Command 'run

(Double' is not supported by Azure Pipelines.



Supported commands

  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@damyanp
Copy link
Member Author

damyanp commented Nov 10, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@damyanp damyanp enabled auto-merge (squash) November 10, 2025 23:53
@damyanp damyanp merged commit eede016 into microsoft:main Nov 11, 2025
12 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in HLSL Roadmap Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants