Enable to playback track 1 session record for track 2 mgmt sdk test cases #11535
Conversation
sdk/core/Azure.Core/tests/TestFramework/DiagnosticScopeValidatingInterceptor.cs
Outdated
Show resolved
Hide resolved
| if (isTrack1Record) | ||
| { | ||
| //there's no domain name for request uri in track 1 record, so add it from reqeust uri | ||
| int len = 8; //length of "https://" |
There was a problem hiding this comment.
are we positive that schema is always https?
There was a problem hiding this comment.
the len we used as start index for searching "/" to get end index of domain name, so 8 is good here even for http://
| { | ||
| if (Mode == RecordedTestMode.Playback && IsTrack1SessionRecord()) | ||
| { | ||
| return _session.Names[callerMethodName].Dequeue(); |
There was a problem hiding this comment.
Who adds items to _session.Names when test is re-recorded?
There was a problem hiding this comment.
for re-record, the record file will be in track 2 format and Names is obsolete.
| @@ -84,7 +98,11 @@ public virtual RecordEntry FindMatch(Request request, IList<RecordEntry> entries | |||
| score++; | |||
| } | |||
|
|
|||
| //we only check Uri + RequestMethod for track1 record | |||
| if (!isTrack1Record) | |||
| { | |||
| score += CompareHeaderDictionaries(headers, entry.Request.Headers, ExcludeHeaders); | |||
There was a problem hiding this comment.
This makes recordings way less useful, validating neither body nor headers.
There was a problem hiding this comment.
This compromise is not ideal, this is best I could figure out, please let me know if any other good suggestion.
|
What's you plan for management plane track 2 recordings? Do you ever plan to re-record tests? what about adding new tests? |
All new tests will be recorded in new format. For existing ones, we plan to do it after finishing higher priority things, like release. |
|
I really have trouble understanding what we are trying to save by reusing the Track 1 recordings, at the point where running tests Live works it's a matter of running 1 command to update the recordings and get higher quality verification with that. I'm worried of this code becoming an instant legacy. |
| int domainEndingIndex = uri.IndexOf('/', len); | ||
| if (domainEndingIndex > 0) | ||
| { | ||
| recordRequestUri = uri.Substring(0, domainEndingIndex) + recordRequestUri; |
There was a problem hiding this comment.
Let's move this logic into RecordEntry.Deserialize this would allow us to drop the additional RequestUri property.
There was a problem hiding this comment.
There's no domain name for request uri in track 1 record, so I get domain name from real request uri and add domain name to RequestUri of track 1 for comparison, the problem is we don't know real request uri during de-serializing. Please let me know if I have some misunderstanding.
The real request uri: https://xxx.com/subscriptions/xxxx
RequestUri in track 1: /subscriptions/xxxxx
RequestUri in track 2: https://xxx.com/subscriptions/xxxx
| @@ -17,6 +17,9 @@ public class RecordEntry | |||
|
|
|||
| public string RequestUri { get; set; } | |||
|
|
|||
| //Used only for deserializing track 1 session record files | |||
| public string EncodedRequestUri { get; set; } | |||
There was a problem hiding this comment.
Let's have an explicit bool IsTrack1Recording property.
There're two reasons to reuse track 1 recordings:
|
267172d to
2cf3255
Compare
The update will support to read track 1 session record for track 2 mgmt sdk test cases in playback mode. In record mode, it will save session record in v2 format.