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

Update LiveTestCtestRegex within the ci.yml to match the CtestRegex value set #6223

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ahsonkhan
Copy link
Member

@ahsonkhan ahsonkhan commented Nov 14, 2024

Before:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=4332699&view=logs&j=b480f431-cfd9-5d07-f486-b73452ab58e9&t=17eb15d2-6ebc-5155-18b3-76202a687fa3

100% tests passed, 0 tests failed out of 100

(no tests skipped)

After:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=4335700&view=logs&j=b480f431-cfd9-5d07-f486-b73452ab58e9&t=1e63cd11-2fcd-5751-b03e-76d95e2e6189

100% tests passed, 0 tests failed out of 100

(no tests skipped)

Conclusion: Explicitly specifying the wild-card isn't necessary here. The regex is already picking up on all the tests that start with azure-data-tables.

All other instances within the repo have the LiveTestCtestRegex value match CtestRegex, so we could consider cleaning that up, for consistency.

Follow-up consistency issue across the repo: #6224
Some use wild-card (*), while others have it wrapped in quotes (""), while the rest leave it as plain-text:
https://github.com/search?q=repo%3AAzure%2Fazure-sdk-for-cpp%20LiveTestCtestRegex&type=code

CtestRegex: "azure-security-keyvault.*"
LiveTestCtestRegex: "azure-security-keyvault.*"

CtestRegex: azure-storage
LineCoverageTarget: 81
BranchCoverageTarget: 47
LiveTestCtestRegex: azure-storage

@ahsonkhan
Copy link
Member Author

/azp run cpp - tables

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ahsonkhan ahsonkhan changed the title Update LiveTestCtestRegex ci.yml to see if it modifies the set of tests that run Update LiveTestCtestRegex within the ci.yml to match the CtestRegex value set Nov 15, 2024
@ahsonkhan ahsonkhan marked this pull request as ready for review November 15, 2024 00:23
@ahsonkhan ahsonkhan added the EngSys This issue is impacting the engineering system. label Nov 15, 2024
@ahsonkhan ahsonkhan self-assigned this Nov 15, 2024
@LarryOsterman
Copy link
Member

FWIW, in yaml, all text elements are treated as strings whether they are quoted or not. The only time you NEED to quote text in yaml is if you need to include special characters in the string (like " or ').

But fundamentally, they are both syntactically correct yaml.

I personally wouldn't stress over the quoting issue in yaml.

Also, the regex format in ctest is described here. The reason you don't need to append .* is that the ctest regex command runs all tests that contain a match for the regular expression - the regular expression does NOT need to encapsulate the entire test name, it just needs to match SOME of the test name

Comment on lines 30 to +33
CtestRegex: azure-data-tables.*
LineCoverageTarget: 77
BranchCoverageTarget: 42
LiveTestCtestRegex: azure-data-tables
LiveTestCtestRegex: azure-data-tables.*
Copy link
Member

Choose a reason for hiding this comment

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

Even if it works without .*, I like that lines 30 and 33 are the same now.

Copy link
Member

Choose a reason for hiding this comment

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

100% agreed with Anton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EngSys This issue is impacting the engineering system. Tables
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants