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.

Sign up for GitHub

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

Jump to bottom

Skip dependency evaluation with --skip-dependencies #3784

Merged
merged 6 commits into from Oct 19, 2023
Merged

Skip dependency evaluation with --skip-dependencies #3784

merged 6 commits into from Oct 19, 2023

Conversation

mdanish-kh
Copy link
Contributor

@mdanish-kh mdanish-kh commented Oct 18, 2023 •
edited

Microsoft Reviewers: Open in CodeFlow

@mdanish-kh mdanish-kh requested a review from a team as a code owner October 18, 2023 13:59
@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Oct 18, 2023
@yao-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft
Copy link
Contributor

InstallerWithDependencies_SkipDependencies test failed because there's an unused override (mock).

By the way, the SkipDependencies arg check should check for user settings like other places as well.

Please let me know if any more info is needed. Thanks.

@mdanish-kh mdanish-kh force-pushed the makeSkipDependenciesSkipDependencies branch from a70a3ee to 4c90716 Compare October 19, 2023 05:16
@mdanish-kh
Copy link
Contributor Author

mdanish-kh commented Oct 19, 2023 •
edited

Please let me know if any more info is needed. Thanks.

Thank you. I added the user setting check in the latest commit.

Can I know how can I locally run/debug the tests defined in WorkflowCommon.cpp? I notice there is a script Run-TestsInPackage.ps1 that sets up the test environment and calls AppInstallerCLITests.exe. I could get those tests to run but not sure how the flow is setup for WorkflowCommon.cpp where the build failure originates from.

Looking at the code, it seems to me that OverrideOpenDependencySource would be getting unused after this change, which I've removed in latest commit, but I can't verify this locally

@yao-msft
Copy link
Contributor

All unit tests are compiled into AppInstallerTests.exe. They are located under src\<arch>\<debug|release>\AppInstallerCliTests

A quick way to run the above test is AppInstallerTests.exe InstallerWithDependencies_SkipDependencies.

@yao-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdanish-kh
Copy link
Contributor Author

All unit tests are compiled into AppInstallerTests.exe. They are located under src\<arch>\<debug|release>\AppInstallerCliTests

A quick way to run the above test is AppInstallerTests.exe InstallerWithDependencies_SkipDependencies.

Thanks! Turns out I did get the test to pass but since it wasn't failing I couldn't see a reference to WorkflowCommon.cpp and unused override error🤦

Looks like InstallerWithDependencies_IgnoreDependenciesSetting test case also requires the same fix

@yao-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft yao-msft merged commit b357fe8 into microsoft:master Oct 19, 2023
8 checks passed
@mdanish-kh mdanish-kh deleted the makeSkipDependenciesSkipDependencies branch October 20, 2023 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yao-msft yao-msft yao-msft approved these changes

Assignees
No one assigned
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

--skip-dependencies does not bypass dependency correlation
2 participants
@mdanish-kh @yao-msft