-
Notifications
You must be signed in to change notification settings - Fork 696
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
Add new test cases for Transitive Package into NuGet.Client-VSDaily Tests pipeline #6148
Open
CiciLi1
wants to merge
7
commits into
dev
Choose a base branch
from
dev-vcicili-addtransitivepackagetestcase
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ccb5b6e
Add transitive package test cases
CiciLi1 765867b
update test name
CiciLi1 e0fea3b
update test name
CiciLi1 c888b50
Update method name
CiciLi1 4df0268
update method name
CiciLi1 39c2427
add search public fields test case
CiciLi1 0fa446f
Update the assert method
CiciLi1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not clear on why this assertion is made both before the
SearchPackageFromUI
call and after the call. Is there a common bug where installing shows the TopLevel package but a following Search does not show that TopLevel package?Should
uiwindow.AssertPackageNameAndType(transitivePackageName, NuGet.VisualStudio.PackageLevel.Transitive);
also be asserted after the Act?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your review. 😊
First question: We call these assertions before
SearchPackageFromUI
to make sure that the package is installed successfully with the top-level package under the “top-level packages” node and the transitive packages under the “transitive packages” node.The second question: the test case is search for packages under the “top-level packages” node, so after searching for top-level packages, only the top-level packages are displayed. So, we didn't add assert transitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I believe also since the search is for an exact match package ID, there should be no transitive shown.
It may be worth considering a test where you search "Contoso" and find both a top-level
Contoso.A
and a transitiveContoso.C
, or similar.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your suggestion, we have added this test case
SearchTopLevelAndTransitivePackagePublicFieldInInstalledTabFromUI
to this PR, please help to review it, thanks again.