Skip to content
This repository was archived by the owner on Jan 21, 2025. It is now read-only.

Tests for ARM, adds ManagedIdentityTrackedResource #609

Merged
merged 13 commits into from
Jul 15, 2024

Conversation

v-hongli1
Copy link
Member

@v-hongli1 v-hongli1 commented Jun 26, 2024

Cadl Ranch Contribution Checklist:

Supported scenarios

  • ManagedIdentityTrackedResourc

    • createWithSystemAssignedOnly(put)
    • updateWithUserAssignedAndSystemAssigne(patch)
    • get
  • I have written a scenario spec

  • I have meaningful @scenario names. Someone can look at the list of scenarios and understand what I'm covering.

  • I have written a mock API

  • I have used @scenarioDocs for extra scenario description and to tell people how to pass my mock api check.

@XiaofeiCao
Copy link
Contributor

Lack one scenario: updateWithUserAssignedOnly
And make sure local live test pass.

@v-hongli1
Copy link
Member Author

v-hongli1 commented Jul 2, 2024

@XiaofeiCao

Lack one scenario: updateWithUserAssignedOnly And make sure local live test pass.

The multiple update cannot be recorded in tsp because the expected path of the scenario is the same.

@pshao25
Copy link
Member

pshao25 commented Jul 3, 2024

@weidongxu-microsoft none (I mean literally none) of the LRO test are supported in .net, because we have a troublesome known issue. Therefore I cannot review.

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Jul 3, 2024

@weidongxu-microsoft none (I mean literally none) of the LRO test are supported in .net, because we have a troublesome known issue. Therefore I cannot review.

what are you talking about...

I asked author to use Sync, not Async.

@pshao25
Copy link
Member

pshao25 commented Jul 3, 2024

@weidongxu-microsoft none (I mean literally none) of the LRO test are supported in .net, because we have a troublesome known issue. Therefore I cannot review.

what are you talking about...

I asked author to use Sync, not Async.

Didn't notice that.

@mcgallan could you have a try and prepare at .net side?
Steps:

  1. Check out this PR.
  2. Run eng\ApplyCadlRanch.ps1 at autorest.csharp.
  3. Add these tests in autorest.csharp to see if there is any problem.
  4. After this PR merged, send out PR for autorest.csharp.

@v-hongli1 v-hongli1 requested a review from pshao25 July 4, 2024 04:56
@v-hongli1 v-hongli1 force-pushed the issues#605_reopen branch from 502ff23 to 0159000 Compare July 4, 2024 09:49
Copy link
Member

@weidongxu-microsoft weidongxu-microsoft left a comment

Choose a reason for hiding this comment

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

You need 2 approvals for cadl-ranch PR.

@XiaofeiCao
Copy link
Contributor

LGTM. Please wait for approval from another language.

Copy link
Member

@msyyc msyyc left a comment

Choose a reason for hiding this comment

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

Looks good to Python. If Java already passes these cases, I think it is OK to merge

Copy link
Member

@weidongxu-microsoft weidongxu-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@weidongxu-microsoft weidongxu-microsoft merged commit af62829 into Azure:main Jul 15, 2024
6 of 7 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants