Skip to content
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

Migrate DateTime.Now to DateTime.UtcNow to make sure all test reporti… #1612

Merged
merged 5 commits into from
May 24, 2018

Conversation

nigurr
Copy link

@nigurr nigurr commented May 22, 2018

Migrate DateTime.Now to DateTime.UtcNow to make sure all test reporting content like trx contains the right data

Issues: https://developercommunity.visualstudio.com/content/problem/196633/tfs-2018-failed-to-publish-test-results-the-value.html

…ng content like trx contains the right data
@@ -272,7 +272,7 @@ internal string WriteEventLogs(List<EventLogEntry> eventLogEntries, int maxLogEn
"{0}-{1}-{2:yyyy}{2:MM}{2:dd}-{2:HH}{2:mm}{2:ss}.{2:fff}",
"Event Log",
Environment.MachineName,
DateTime.Now);
DateTime.UtcNow);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add tests for these?

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@singhsarab singhsarab left a comment

Choose a reason for hiding this comment

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

Please fix comments.

@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Xml.Linq;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This should go under namespace.

Copy link
Author

Choose a reason for hiding this comment

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

done

@@ -36,8 +36,8 @@ public TestResult(TestCase testCase)

// Default start and end time values for a test result are initialized to current timestamp
// to maintain compatibility.
this.StartTime = DateTimeOffset.Now;
this.EndTime = DateTimeOffset.Now;
this.StartTime = DateTimeOffset.UtcNow;
Copy link
Contributor

Choose a reason for hiding this comment

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

This could change the behavior for inner loop scenarios. If any client displaying time it's client responsibility to localize the date. TestPlatform will use UTC for consistence.

Copy link
Author

Choose a reason for hiding this comment

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

As we discussed, it might be. But we need to draw this and follow everywhere as UTC standard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, Totally agreed.

@nigurr nigurr merged commit e2df268 into microsoft:master May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants