-
Notifications
You must be signed in to change notification settings - Fork 325
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
Blame Sequence File Changes #1716
Blame Sequence File Changes #1716
Conversation
@kaadhina Please review these changes |
/// </summary> | ||
/// <returns>Guid test id</returns> | ||
private Guid GetTestId() | ||
{ |
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.
remove
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.
Removed this
@@ -155,7 +155,7 @@ private void EventsTestCaseStart(object sender, TestCaseStartEventArgs e) | |||
EqtTrace.Info("Blame Collector : Test Case Start"); | |||
} | |||
|
|||
this.testSequence.Add(e.TestElement); | |||
this.testSequence.Add(new BlameTestObject(e.TestElement)); | |||
this.testStartCount++; |
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.
make dict
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.
Dictionary and list used.
@@ -55,7 +55,7 @@ protected XmlReaderWriter(IFileHelper fileHelper) | |||
/// The file Path. | |||
/// </param> | |||
/// <returns>File path</returns> | |||
public string WriteTestSequence(List<TestCase> testSequence, string filePath) |
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.
TestCase [](start = 45, length = 8)
check for outcome/result
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.
There is no outcome field in TestCase object.
@@ -198,7 +198,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab | |||
|
|||
// Setup | |||
this.mockProcessDumpUtility.Setup(x => x.GetDumpFile()).Returns(this.filepath); | |||
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<TestCase>>(), It.IsAny<string>())) | |||
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<BlameTestObject>>(), It.IsAny<string>())) |
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.
UTs for both completed and not completed
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.
Added UTs in XmlReaderWriterTests.cs
// Add the test object to the dictionary. | ||
this.testObjectDictionary.Add(blameTestObject.Id, blameTestObject); | ||
|
||
// Increment test start count. | ||
this.testStartCount++; |
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.
this.testStartCount++; [](start = 12, length = 22)
Do we still need this>?
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.
We will have to check for entire dictionary if any test remains incomplete to decide if sequence is to be written to file. This makes an easier check.
var testCaseList = xmlReaderWriter.ReadTestSequence(filePath); | ||
File.Delete(filePath); | ||
|
||
Assert.AreEqual(testCaseList.First().FullyQualifiedName, "Abc.UnitTest1"); |
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.
Displayname check?
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.
Will add that.
.Returns(this.filepath); | ||
|
||
this.mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(testcase)); | ||
this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext)); | ||
|
||
// Verify WriteTestSequence Call | ||
this.mockBlameReaderWriter.Verify(x => x.WriteTestSequence(It.IsAny<List<TestCase>>(), It.IsAny<string>()), Times.Once); | ||
this.mockBlameReaderWriter.Verify(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()), Times.Once); | ||
} |
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.
validation for values in the dictionary and list, we should validate that iscompleted, fqdn, display name etc were set properly
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.
Added validations for dictionary and list.
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.
@dotnet-bot test this please |
Blame Sequence File Changes
Added Display name and Is Test Completed in the sequence file that is generated by BlameDataCollector