Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Tue, 18 May 2021 23:22:49 GMT, Stephen Colebourne wrote: >> test/jdk/java/time/test/java/time/TestInstantSource.java line 86: >> >>> 84: assertEquals(test.instant(), instant); >>> 85: assertSame(test.equals(InstantSource..fixed(instant)); >>> 86: assertEquals(test.hashCode(), >>> InstantSource..fixed(instant).hashCode()); >> >> Not sure this would compile. > > I was expecting the GitHub Action to pickup coding and test issues (since my > dev setup can't compile or run tests). But it seems it doesn't. do that. The > latest commit should at least compile as I copied the test class to another > IDE location, but I have no way of knowing if the tests pass. GitHub Actions just verifies build and tier1 testing, while java.time tests are in tier2, so it won't catch the failure. The test case still does not compile without the `import java.time.InstantSource` statement. I've verified the test passed with that fix. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Tue, 18 May 2021 21:04:18 GMT, Naoto Sato wrote: >> Stephen Colebourne has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8266846: Add java.time.InstantSource > > test/jdk/java/time/test/java/time/TestInstantSource.java line 86: > >> 84: assertEquals(test.instant(), instant); >> 85: assertSame(test.equals(InstantSource..fixed(instant)); >> 86: assertEquals(test.hashCode(), >> InstantSource..fixed(instant).hashCode()); > > Not sure this would compile. I was expecting the GitHub Action to pickup coding and test issues (since my dev setup can't compile or run tests). But it seems it doesn't. do that. The latest commit should at least compile as I copied the test class to another IDE location, but I have no way of knowing if the tests pass. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Mon, 17 May 2021 17:19:19 GMT, Stephen Colebourne
wrote:
>> 8266846: Add java.time.InstantSource
>
> Stephen Colebourne has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8266846: Add java.time.InstantSource
src/java.base/share/classes/java/time/Instant.java line 275:
> 273: */
> 274: public static Instant now() {
> 275: return Clock.currentInstant();
Copyright year should change to 2021 for this change.
test/jdk/java/time/test/java/time/TestInstantSource.java line 59:
> 57: assertTrue(Math.abs(testMillis - millis) < 1000);
> 58: assertTrue(Math.abs(testInstantMillis - millis) < 1000);
> 59: assertSame(test.equals(InstantSource.system());
')' is missing. Couple other locations.
test/jdk/java/time/test/java/time/TestInstantSource.java line 86:
> 84: assertEquals(test.instant(), instant);
> 85: assertSame(test.equals(InstantSource..fixed(instant));
> 86: assertEquals(test.hashCode(),
> InstantSource..fixed(instant).hashCode());
Not sure this would compile.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Mon, 17 May 2021 17:19:19 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource Changes requested by plevart (Reviewer). src/java.base/share/classes/java/time/Clock.java line 487: > 485: // it more unlikely to hit the 1ns in the future condition. > 486: localOffset = System.currentTimeMillis()/1000 - 1024; > 487: Is it possible that after a fresh localOffset is retrieved, the thread is preempted and when it is scheduled again after a pause, the getNanoTimeAdjustment below returns -1 ? Would it help if instead of throwing exception, there was an infinite retry loop? - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Mon, 17 May 2021 17:13:58 GMT, Stephen Colebourne
wrote:
>> src/java.base/share/classes/java/time/Clock.java line 128:
>>
>>> 126: * Implementations should implement {@code Serializable} wherever
>>> possible and must
>>> 127: * document whether or not they do support serialization.
>>> 128: *
>>
>> The ImplSpec needs to say how it is implemented.
>> The 'implements InstantSource' can not mandate any particular
>> implementation. Its just an interface the real behavior comes from its
>> implementations. In this case Clock. Referring to the static methods of
>> InstantSource behavior may be sufficient because that behavior is concrete.
>
> There are plenty of examples of interfaces in `java.time` and elsewhere that
> apply restrictions to implementations. Nevertheless, for simplicity and
> expediency I have reinstated the `implSpec` on `Clock`
ok, thanks.
Assertions in interfaces are toothless. Tests can only be written for
implementations.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Mon, 17 May 2021 14:30:20 GMT, Roger Riggs wrote:
>> Stephen Colebourne has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8266846: Add java.time.InstantSource
>
> src/java.base/share/classes/java/time/Clock.java line 128:
>
>> 126: * Implementations should implement {@code Serializable} wherever
>> possible and must
>> 127: * document whether or not they do support serialization.
>> 128: *
>
> The ImplSpec needs to say how it is implemented.
> The 'implements InstantSource' can not mandate any particular implementation.
> Its just an interface the real behavior comes from its implementations. In
> this case Clock. Referring to the static methods of InstantSource behavior
> may be sufficient because that behavior is concrete.
There are plenty of examples of interfaces in `java.time` and elsewhere that
apply restrictions to implementations. Nevertheless, for simplicity and
expediency I have reinstated the `implSpec` on `Clock`
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
> 8266846: Add java.time.InstantSource Stephen Colebourne has updated the pull request incrementally with one additional commit since the last revision: 8266846: Add java.time.InstantSource - Changes: - all: https://git.openjdk.java.net/jdk/pull/4016/files - new: https://git.openjdk.java.net/jdk/pull/4016/files/befc7621..7fa9d0ec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4016=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4016=00-01 Stats: 38 lines in 2 files changed: 31 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4016.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4016/head:pull/4016 PR: https://git.openjdk.java.net/jdk/pull/4016
