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

Unable to compare some timestamp types #3359

Open
IUSR opened this issue Feb 7, 2024 · 3 comments · May be fixed by #3467
Open

Unable to compare some timestamp types #3359

IUSR opened this issue Feb 7, 2024 · 3 comments · May be fixed by #3467
Assignees
Labels
type: bug A general bug
Milestone

Comments

@IUSR
Copy link

IUSR commented Feb 7, 2024

Describe the bug
A clear and concise description of what the bug is.

  • assertj core version: 3.25.3
  • java version: 21.0.1-open
  • test framework version: JUnit 5.10.2
  • os (if relevant): Linux 6.5.0-15-generic # 15-Ubuntu SMP PREEMPT_DYNAMIC Tue Jan 9 17:03:36 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux (Ubuntu 23.10)

Test case reproducing the bug

Some timestamp types, for example, java.sql.Timestamp, are not specifically handled when comparing with other temporal types, and as a result, though there are no compile-time errors caused by types, they are not effectively compared as temporals.

public class Demo {
    @Test
    void compareSqlTimestampWithInstant() {
        final long now = System.currentTimeMillis();
        final Timestamp ts = new Timestamp(now);
        final Instant instant = Instant.ofEpochMilli(now);
        assertEquals(ts.getNanos(), instant.getNano()); // this succeeds
        assertThat(ts).isEqualTo(instant); // this fails.
        assertThat(ts).isAfterOrEqualTo(instant); // even if I want to relax the check, this still fails
    }
}

I can see there are javadocs mentioning that comparisons are done by converting these timestamp types to java.util.Date, but is there a way to make this less surprising, otherwise make it a compilation error, so the user can take care of nanoseconds?

Updated: I thought it was caused by nano-second precision being dropped along the assertion classes, but it turns out that there is no special handling of such types, and they end up calling Object.equals().

@IUSR IUSR changed the title Nanoseconds dropped when comparing types that support nanosecond precision Unable to compare some timestamp types Feb 7, 2024
@scordio
Copy link
Member

scordio commented Feb 8, 2024

Thanks for reporting it, @IUSR.

The root cause is the usage of Date.from(Instant) internally, which doesn't behave correctly when actual is a Timestamp.

As a workaround, you can rely on Timestamp.from(Instant):

assertThat(ts).isEqualTo(Timestamp.from(instant));
assertThat(ts).isAfterOrEqualTo(Timestamp.from(instant));

@scordio scordio added the type: bug A general bug label Feb 8, 2024
@scordio scordio added this to the 3.26.0 milestone Feb 8, 2024
@IUSR
Copy link
Author

IUSR commented Feb 9, 2024

Thanks for confirming, Stefano! Yes, I am currently converting the timestamp/temporal objects into the same type and comparing them as a workaround. It'd be good when things "just work", as my experience with assertj has been :)

@scordio
Copy link
Member

scordio commented May 9, 2024

I'm afraid we didn't fully solve the issue.

Although the original example now works, a case with non-zero nanoseconds fails:

Instant instant = Instant.ofEpochSecond(0, 1); // 1970-01-01T00:00:00.000000001Z
Timestamp timestamp = Timestamp.from(instant);
assertThat(timestamp).isEqualTo(instant); // fails

I'll reopen it and have a look.

@scordio scordio reopened this May 9, 2024
@scordio scordio self-assigned this May 9, 2024
@scordio scordio linked a pull request May 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
2 participants