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

Fix for wrong start time #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cabauman
Copy link

@cabauman cabauman commented May 14, 2020

The performance testing extension seems to output the Date field in seconds, rather than milliseconds. I also added a call to ToLocalTime to get correct results for different time zones.

Before:
before

After:
after

resolves #4

I read the contributing guidelines but I don't see a CHANGELOG.MD file in this repository. Let me know if I need to do any modifications or extra steps.

EDIT
Hold off on this until I can update the failing unit tests.

The problem is that the xml result files that the unit tests use have a different format than what the latest Performance testing API exports (e.g. millisecond vs second timestamps). So maybe the xml result files in this repo are out of date? The tests pass when I use newly exported result files.

@seanstolberg-unity
Copy link
Contributor

Yes, that result file in the unit tests is out of date. Thanks for putting this fix in. Does it still pass on the xml result file from the xrautomated tests repo, the baseline file? If you could check that, I would be good merging this.

@cabauman
Copy link
Author

cabauman commented May 14, 2020

Actually, the XRAutomated repo also uses the older xml format just like the result files for unit tests here, so those don't work.

Older format:
##performancetestruninfo includes the timestamp like StartTime":1586641593946.3528

Newer format:
##performancetestruninfo2 includes the timestamp like "Date":1589467385

But now that I think about it, a better fix than changing AddMilliseconds to AddSeconds is doing

StartTime = TimeSpan.FromSeconds(result.Date).TotalMilliseconds,

in TestResultXmlParser.cs in the DeserializeMetadataV2 method. Then v2 format will be in milliseconds by the time it gets to result processor.

Just tried it out and works for both formats! It's after midnight where I'm at, so I'll wait until tomorrow to make that modification, along with a few additional unit tests. Does the new proposed fix sound okay?

@cabauman
Copy link
Author

Since @gintautasss modified the performance testing package to output Date in milliseconds, my fix here is no longer needed (so I got rid of that commit), except for the ToLocalTime part. In addition to that small change, I went ahead and added a few of the newer xml format result files, along with tests. Let me know if there's anything else I need to do.

cds-mirroring-service-user pushed a commit that referenced this pull request Apr 7, 2022
Update PBR to support Json format performance test run files
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.

Wrong TestResults date
2 participants