Skip to content

Produce a better message when test fails#5676

Merged
findepi merged 5 commits intotrinodb:masterfrom
findepi:findepi/test-zone-improvements
Oct 24, 2020
Merged

Produce a better message when test fails#5676
findepi merged 5 commits intotrinodb:masterfrom
findepi:findepi/test-zone-improvements

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Oct 23, 2020

Before the change the test fails a message that was hard to understand for me:

java.lang.AssertionError: expected [174262] but found [172032]
    Expected :174262
    Actual   :172032

the change introduces a better exception message but also cleans up the test code, which was unnecessarily involving java.util.TimeZone.

@cla-bot cla-bot bot added the cla-signed label Oct 23, 2020
@findepi findepi requested a review from sopel39 October 23, 2020 12:07
Previously the test would fail with something like

    java.lang.AssertionError: expected [174262] but found [172032]
    Expected :174262
    Actual   :172032
@findepi findepi force-pushed the findepi/test-zone-improvements branch from b843a61 to 7108b41 Compare October 23, 2020 14:11
@findepi findepi requested a review from martint October 23, 2020 14:11
<violation>
<name>org/joda/time/DateTimeZone.toTimeZone:()Ljava/util/TimeZone;</name>
<version>1.8</version>
<comment>Avoid DateTimeZone.toTimeZone as it returns GMT for a zone not supported by the JVM. Use TimeZone.getTimeZone(ZoneId.of(dtz.getId())) instead.</comment>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as it returns GMT for a zone not supported by the JVM

wow!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I will likewise block use of TimeZone.forTimeZone(String).

Avoid TimeZone.getTimeZone as it returns GMT for a zone not supported
by the JVM.
@findepi findepi requested review from martint and sopel39 October 23, 2020 17:19
@findepi findepi merged commit 5553f58 into trinodb:master Oct 24, 2020
@findepi findepi deleted the findepi/test-zone-improvements branch October 24, 2020 20:39
aweisberg added a commit to aweisberg/presto that referenced this pull request Feb 23, 2021
Cherrypick of trinodb/trino#4463
Cherrypick of trinodb/trino#5676

Co-authored-by: David Phillips <david@acz.org>
Co-authored-by: Piotr Findeisen <piotr.findeisen@gmail.com>
arhimondr pushed a commit to prestodb/presto that referenced this pull request Feb 24, 2021
Cherrypick of trinodb/trino#4463
Cherrypick of trinodb/trino#5676

Co-authored-by: David Phillips <david@acz.org>
Co-authored-by: Piotr Findeisen <piotr.findeisen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants