search

Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
114 stars 180 forks source link

[FEATURE REQ] Compare JSON Content By Key/Value, Not Text Diff #7477

Open archerzz opened 2 years ago

archerzz commented 2 years ago

Library name

Azure.Core.TestFramework

Please describe the feature.

Description

Looks like in the playback mode, the test framework is comparing JSON content by text diff, which will cause false alerts when only the sequence of some properties has been changed.

Example

We see repetitive alerts below after the SDK codes are refreshed.

Azure.Core.TestFramework.TestRecordingMismatchException : Unable to find a record for the request PUT https://management.azure.com/subscriptions/xxxxx/resourcegroups/teststorageRG-5248?api-version=2021-04-01
Header differences:
Body differences:
Request and response bodies do not match at index 2:
     request: "{"tags":{"test":"env"}"
     record:  "{"location":"eastus2","

But actually the content is not changed semantically.

MicrosoftTeams-image

Solution

We should try to compare the JSON body content by key/value, not the text diff.

azure-sdk commented 2 years ago

Label prediction was below confidence level 0.6 for Model:ServiceLabels: 'Search:0.11814453,Storage:0.101195894,Azure.Core:0.100758575'

JoshLove-msft commented 2 years ago

Hi @archerzz, Is there a reason that the order changed? Was there a code change that went along with this? Assuming that is the case, we'd normally recommend just re-recording for situations like this. While it may be possible to do what you suggest here, the serialization format is expected to be stable and unexpected changes should be flagged.

JoshLove-msft commented 2 years ago

To make re-recording easier, you can use attribute your tests with the RecordedTest attribute, which will automatically re-record for you on playback failure.

archerzz commented 2 years ago

Hi @JoshLove-msft I think some code change in the autorest.csharp caused the order change.

Re-recording should fix the issue. The only pain point is that some recording will take hours. So I filed this issue as feature request, which can reduce the chances to do a re-recording.

This is not a blocker.

pallavit commented 11 months ago

@JoshLove-msft Given that we have moved to the proxy, is this still a valid concern?

JoshLove-msft commented 11 months ago

The concern is still valid, but this would need to be addressed in the Test Proxy so I can move it to the tools repo.