Send SessionRecords to correct location by nisha-bhatia · Pull Request #15937 · Azure/azure-sdk-for-net

nisha-bhatia · 2020-10-13T20:27:14Z

JoshLove-msft · 2020-10-14T03:12:12Z

            string fileName = name + (IsAsync ? "Async" : string.Empty) + ".json";
-            return Path.Combine(TestContext.CurrentContext.TestDirectory,
+
+            var path = TestContext.CurrentContext.TestDirectory;


This is an enhancement that we would want to enable for all Track 2 tests. It will be really helpful for users.

It might be better to use an attribute that we include in https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/Directory.Build.props.

<ItemGroup> <AssemblyAttribute Include="System.Reflection.AssemblyMetadataAttribute"> <_Parameter1>SourcePath</_Parameter1> <_Parameter2>$(MSBuildProjectDirectory)</_Parameter2> </AssemblyAttribute> </ItemGroup>

We can then retrieve the path like this from within RecordedTestBase:

string path = ((AssemblyMetadataAttribute) GetType().Assembly.GetCustomAttribute(typeof(AssemblyMetadataAttribute))).Value;

We were only going to do this for MGMT plane, are you suggesting we do this for data plane as well?

This suggestion seems simpler as well if @nisha-bhatia can get it from this property.

Also, we were going to apply this to track 1 MGMT as well, not just track 2.

I think making this change both for data and management plane makes sense. There is no need to bifurcate the workflow.

JoshLove-msft · 2020-10-14T03:22:00Z

We should remove the UpdateSessionRecords target and update all of our documentation that references it:
https://github.com/Azure/azure-sdk-for-net/search?q=UpdateSessionRecords

pakrym · 2020-10-14T15:37:29Z

+                rpname = testname.Substring(rpnamefirstindex, testname.Length - rpnamefirstindex).ToLower();
+                rpname.Replace(".", "-");
+            }
+            else if (testname.Substring(0,9) == "Management") //Track 1


This test framework is not used for track 1 projects.

…s, remove Track 1, add isMgmtProject attribute to track 2 rps

pakrym · 2020-10-27T15:02:59Z

 ## Recording

-When tests are run in recording mode, session records are saved to `artifacts/bin/<ProjectName>/<TargetFramework>/SessionRecords` directory. You can copy recordings to the project directory manually or by executing `dotnet msbuild /t:UpdateSessionRecords` in the test project directory.
+When tests are run in recording mode, session records are saved to the project directory manually in a folder named 'Session Records'.


Suggested change

When tests are run in recording mode, session records are saved to the project directory manually in a folder named 'Session Records'.

When tests are run in recording mode, session records are saved to the project directory manually in a folder named 'SessionRecords'.

pakrym · 2020-10-27T15:03:27Z

 }

-dotnet msbuild /t:UpdateSessionRecords
+dotnet msbuild


This is not required

@vinagesh fyi

pakrym · 2020-10-27T15:18:20Z

azure-sdk-for-net/eng/TestFramework.targets

Line 6 in 15174b1

    
           <Target Name="AutoUpdateSessionRecords" Condition="'$(AutoUpdateSessionRecords)' == 'true'" DependsOnTargets="UpdateSessionRecordsInner" AfterTargets="Test;VSTest"/>

and
https://github.com/Azure/azure-sdk-for-net/blob/master/eng/pipelines/templates/jobs/archetype-sdk-tests.yml#L145 needs removing as well.

pakrym · 2020-10-27T15:20:04Z

@@ -5,11 +5,6 @@

  <Target Name="AutoUpdateSessionRecords" Condition="'$(AutoUpdateSessionRecords)' == 'true'" DependsOnTargets="UpdateSessionRecordsInner" AfterTargets="Test;VSTest"/>


This can be removed as well as UpdateSessionRecordsInner target.

pakrym · 2020-10-27T21:06:24Z

 }

-dotnet msbuild /t:UpdateSessionRecords
+dotnet msbuild


All of these dotnet msbuild invocations are not required

pakrym · 2020-10-27T22:20:38Z

@@ -1 +1 @@
-dotnet msbuild /t:UpdateSessionRecords
+dotnet msbuild


This file is not required as well.

This reverts commit 344ef94.

bquantump · 2020-10-28T01:14:47Z

This appears to be a flakey test as rerunning the failed test allowed it to pass @pakrym
Rerun: https://github.com/Azure/azure-sdk-for-net/runs/1318105030

2020-10-27T23:02:37.0825692Z X AddsDelay [129ms]
2020-10-27T23:02:37.0825955Z Error Message:
2020-10-27T23:02:37.0826248Z Expected: less than or equal to 78
2020-10-27T23:02:37.0826554Z But was: 80
2020-10-27T23:02:37.0826690Z
2020-10-27T23:02:37.0827022Z Stack Trace:
2020-10-27T23:02:37.0827411Z at Azure.Search.Documents.Tests.ManualRetryDelayTests.AddsDelay() in /home/vsts/work/1/s/sdk/search/Azure.Search.Documents/tests/Batching/ManualRetryDelayTests.cs:line 29
2020-10-27T23:02:37.0828094Z
2020-10-27T23:02:37.0828461Z âˆš AddsMoreDelay [312ms]

This reverts commit 17b964f.

pakrym · 2020-10-28T14:49:29Z

Yes, feel free to file failing tests. This one is already filed at #16291

* send SessionRecords to correct location * add more error handling * WIP: added dataplane and Track 2 correct locations for Session Records, remove Track 1, add isMgmtProject attribute to track 2 rps * Update Directory.Build.props * Update RecordedTestBase.cs * add IsTestProject * remove changes to eventhub * remove unnecessary imports * update docs and remove UpdateSessionRecords target * update documentation and targets * forgot to remove dotnet build * Delete UpdateSessionRecords.ps1 Co-authored-by: bquantump <53361486+bquantump@users.noreply.github.com>

nisha-bhatia added 2 commits October 13, 2020 13:20

send SessionRecords to correct location

2f35cb2

Merge branch 'updateRecordingsLocation'

1a2e40e

nisha-bhatia requested review from JoshLove-msft, YalinLi0312, allenjzhang, bquantump, m-nash, markcowl and pakrym October 13, 2020 20:27

nisha-bhatia requested a review from KrzysztofCwalina as a code owner October 13, 2020 20:27

nisha-bhatia self-assigned this Oct 13, 2020

ghost added the Azure.Core label Oct 13, 2020

nisha-bhatia removed the request for review from KrzysztofCwalina October 13, 2020 20:27

bquantump reviewed Oct 13, 2020

View reviewed changes

Comment thread sdk/core/Azure.Core.TestFramework/src/RecordedTestBase.cs Outdated

bquantump changed the title ~~Send SessionRecords to correct location~~ Send SessionRecords to correct location for MGMT Oct 13, 2020

nisha-bhatia added 2 commits October 13, 2020 17:06

add more error handling

1135d95

Merge branch 'updateRecordingsLocation'

2d4ac5a

bquantump reviewed Oct 14, 2020

View reviewed changes

Comment thread sdk/core/Azure.Core.TestFramework/src/RecordedTestBase.cs Outdated

bquantump reviewed Oct 14, 2020

View reviewed changes

Comment thread sdk/core/Azure.Core.TestFramework/src/RecordedTestBase.cs Outdated

JoshLove-msft reviewed Oct 14, 2020

View reviewed changes

pakrym reviewed Oct 14, 2020

View reviewed changes

nisha-bhatia added 2 commits October 19, 2020 18:19

WIP: added dataplane and Track 2 correct locations for Session Record…

97f998d

…s, remove Track 1, add isMgmtProject attribute to track 2 rps

Merge branch 'updateRecordingsLocation'

b134864

nisha-bhatia requested a review from AlexGhiondea as a code owner October 20, 2020 01:21

nisha-bhatia removed the request for review from AlexGhiondea October 20, 2020 01:21

JoshLove-msft reviewed Oct 20, 2020

View reviewed changes

Comment thread sdk/core/Azure.Core.TestFramework/src/RecordedTestBase.cs Outdated

JoshLove-msft reviewed Oct 20, 2020

View reviewed changes

Comment thread sdk/core/Azure.Core.TestFramework/src/RecordedTestBase.cs Outdated

nisha-bhatia added 2 commits October 20, 2020 00:19

remove checking if rp is mgmt or dataplane

3495ed0

Merge branch 'updateRecordingsLocation'

ce1ce19

nisha-bhatia requested review from barustum, bikamani, chidozieononiwu, danieljurek, drwill-ms, mitchdenny, prmathur-microsoft, timtay-microsoft, vinagesh and weshaggard as code owners October 27, 2020 06:59

pakrym reviewed Oct 27, 2020

View reviewed changes

pakrym approved these changes Oct 27, 2020

View reviewed changes

pakrym reviewed Oct 27, 2020

View reviewed changes

update documentation and targets

0dfef67

bquantump approved these changes Oct 27, 2020

View reviewed changes

pakrym reviewed Oct 27, 2020

View reviewed changes

pakrym approved these changes Oct 27, 2020

View reviewed changes

JoshLove-msft approved these changes Oct 27, 2020

View reviewed changes

forgot to remove dotnet build

344ef94

pakrym reviewed Oct 27, 2020

View reviewed changes

nisha-bhatia added 2 commits October 27, 2020 15:45

Delete UpdateSessionRecords.ps1

17b964f

Revert "forgot to remove dotnet build"

7f04484

This reverts commit 344ef94.

nisha-bhatia added 2 commits October 27, 2020 18:15

Revert "Delete UpdateSessionRecords.ps1"

6295a38

This reverts commit 17b964f.

Delete UpdateSessionRecords.ps1

b15f7f8

bquantump merged commit d850637 into Azure:master Oct 28, 2020

	When tests are run in recording mode, session records are saved to the project directory manually in a folder named 'Session Records'.
	When tests are run in recording mode, session records are saved to the project directory manually in a folder named 'SessionRecords'.

		@@ -5,11 +5,6 @@

		<Target Name="AutoUpdateSessionRecords" Condition="'$(AutoUpdateSessionRecords)' == 'true'" DependsOnTargets="UpdateSessionRecordsInner" AfterTargets="Test;VSTest"/>

		@@ -1 +1 @@
		dotnet msbuild /t:UpdateSessionRecords No newline at end of file
		dotnet msbuild No newline at end of file

Conversation

nisha-bhatia commented Oct 13, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bquantump Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pakrym Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoshLove-msft commented Oct 14, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pakrym commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bquantump commented Oct 28, 2020

Uh oh!

pakrym commented Oct 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bquantump Oct 14, 2020 •

edited

Loading

pakrym Oct 14, 2020 •

edited

Loading

pakrym commented Oct 27, 2020 •

edited

Loading