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

Add year, minute to the --diagnostic filename #3751

Closed
AliveDevil opened this issue Sep 3, 2024 · 5 comments · Fixed by #3894
Closed

Add year, minute to the --diagnostic filename #3751

AliveDevil opened this issue Sep 3, 2024 · 5 comments · Fixed by #3894
Assignees
Labels
Area: Testing Platform Belongs to the Microsoft.Testing.Platform core library In-PR Type: Feature

Comments

@AliveDevil
Copy link

AliveDevil commented Sep 3, 2024

Describe the bug

The --diagnostic-option creates files with an opaque, non-modified date sorting friendly file name.

Steps To Reproduce

  1. Run multiple successive tests with --diagnostic, when process Ids are recycled, the name sorting option is useless.

Expected behavior

Some name sorting friendly file prefix (i.e. log_yyyymmddThhmmss_ProcessId.diag).

Actual behavior

Screenshot_20240903_125137

log_09030958760.diag: 11:47
log_09031011248.diag: 12:30
log_09031028245.diag: 12:34
log_09031043246.diag: 12:17
log_09031043969.diag: 12:10
log_09031045242.diag: 12:34
log_09031055355.diag: 12:09
@nohwnd nohwnd added Type: Feature Area: Testing Platform Belongs to the Microsoft.Testing.Platform core library and removed Needs: Triage 🔍 labels Sep 3, 2024
@nohwnd nohwnd added this to the MSTest 3.7 / Platform 1.5 milestone Sep 3, 2024
@nohwnd
Copy link
Member

nohwnd commented Sep 3, 2024

Adding to next milestone, looks like low hanging fruit, and I don't think change of name in diag log should be considered a breaking change.

@MarcoRossignoli MarcoRossignoli self-assigned this Sep 5, 2024
@MarcoRossignoli
Copy link
Contributor

MarcoRossignoli commented Sep 5, 2024

? Some name sorting friendly file prefix (i.e. log_yyyymmddThhmmss_ProcessId.diag)

Fine to me a format like that, keeping in mind that you should not take the "process_id" as guarantee for nothing, that id can be reused for different processes, maybe we can simply have time ordering, anyway today we have a retry loop if the file is already taken by someone else in the same folder so the order will be preserved and the pid says nothing about the nature of the file, maybe we can add the tfm the make easy to read it.

@MarcoRossignoli
Copy link
Contributor

@AliveDevil I did a check of the code and we're creating the file with the pattern

$"{_options.LogPrefixName}_{_clock.UtcNow.ToString("MMddHHssfff", CultureInfo.InvariantCulture)}.diag";

You can change the prefix using the option --diagnostic-output-fileprefix as explained here https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-platform-intro?tabs=dotnetcli#options

Now the logs are already "ordered", it's not clear to me what's the difference if we change the file name pattern.

Can you elaborate why the current stable ordering is not enough? You'll have always the log files ordered and adding the PID doesn't change much, the time granularity is small enough to make it useless for ordering scope.

@AliveDevil
Copy link
Author

AliveDevil commented Sep 30, 2024

When the format really is:

  • Two Digit Month
  • Two Digit Day
  • Two Digit Hour
  • Two Digit Seconds
  • Three Digits Milliseconds

I'm missing a two digit minutes field there.

Thus a diagnostics log for 2024-30-09 01:00:59.000 is listed after a diagnostics log for 2024-30-09 01:59:00.000 which doesn't make sense and is unintuitive. Based on the format above:

  • 09300100000 for the 58 minutes later 2024-30-09 01:59:00.000
  • 09300159000 for the top-of-the-hour 2024-30-09 01:00:59.000

I thought it was the PID, because it was just random garbage which didn't end up making any sense when combined with the explorer reported date (and five digit process ids arent that uncommon).

@MarcoRossignoli
Copy link
Contributor

MarcoRossignoli commented Sep 30, 2024

Thus a diagnostics log for 2024-30-09 01:00:59.000 is listed after a diagnostics log for 2024-30-09 01:59:00.000 which doesn't make sense and is unintuitive. Based on the format above:

Ok now it's clear my bad, we can add back the minute(we wanted to have short name as possible), usually I don't expect users will parse the filename and the creation time should be used for the ordering(done by OS and preserved with the copy).

@MarcoRossignoli MarcoRossignoli changed the title --diagnostic-log not sort-by-name friendly Add minute to the --diagnostic filename Sep 30, 2024
@MarcoRossignoli MarcoRossignoli changed the title Add minute to the --diagnostic filename Add year, minute to the --diagnostic filename Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Testing Platform Belongs to the Microsoft.Testing.Platform core library In-PR Type: Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants