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

dump timezone and offset for junit reporter xml timestamp #2388

Closed
wants to merge 1 commit into from

Conversation

graingert
Copy link

@graingert graingert commented Aug 20, 2020

Overview

related to pytest-dev/pytest#7662

It looks like pytest (and Junit) just takes the system time from OS, and save it to test report.
Unfortunately, this timestamp doesn't have any timezone information, Our CI system will treat this timestamp as a UTC time, even the timestamp is actually from another time zone.


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@sbrannen
Copy link
Member

Can you please provide a rationale for the proposed change?

@graingert
Copy link
Author

@sbrannen updated

@sbrannen
Copy link
Member

Note that your PR breaks the build with failing tests.

Before submitting a PR, please make sure you execute ./gradlew build locally.

@graingert graingert marked this pull request as draft August 20, 2020 12:45
@marcphilipp
Copy link
Member

Is passing -Duser.timezone=UTC to the test JVM a viable workaround?

@codecov
Copy link

codecov bot commented Aug 20, 2020

Codecov Report

Merging #2388 into main will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2388      +/-   ##
============================================
- Coverage     89.86%   89.85%   -0.01%     
- Complexity     4544     4545       +1     
============================================
  Files           397      397              
  Lines         11238    11239       +1     
  Branches        909      909              
============================================
  Hits          10099    10099              
  Misses          863      863              
- Partials        276      277       +1     
Impacted Files Coverage Δ Complexity Δ
...g/junit/platform/engine/reporting/ReportEntry.java 100.00% <100.00%> (ø) 8.00 <1.00> (+1.00)
...platform/reporting/legacy/xml/XmlReportWriter.java 96.53% <100.00%> (ø) 56.00 <1.00> (ø)
...jupiter/engine/execution/ExtensionValuesStore.java 89.70% <0.00%> (-1.48%) 21.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4688072...c55a378. Read the comment docs.

@graingert graingert marked this pull request as ready for review August 20, 2020 17:23
@@ -317,10 +316,11 @@ void writesHostNameAndTimestamp(@TempDir Path tempDirectory) throws Exception {
engine.addTest("test", () -> {
});

LocalDateTime now = LocalDateTime.parse("2016-01-28T14:02:59.123");
ZoneId zone = ZoneId.systemDefault();
String timestamp = "2016-01-28T14:02:59Z[Europe/London]";
Copy link
Member

Choose a reason for hiding this comment

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

❓ That's the format you want in the XML? Could you please double check that tools that read this XML report, e.g. Jenkins, don't choke on this?

Copy link
Author

Choose a reason for hiding this comment

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

Yep going to do the same for pytest too pytest-dev/pytest#7662 (comment)

@stale
Copy link

stale bot commented May 13, 2021

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please reopen the PR. Thank you for your contributions.

@stale stale bot added the status: stale label May 13, 2021
@stale
Copy link

stale bot commented Jun 3, 2021

This pull request has been automatically closed due to inactivity. If you are still interested in contributing this, please ensure that it is rebased against the latest branch (usually main), all review comments have been addressed and the build is passing.

@stale stale bot closed this Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants