8327007: javax/swing/JSpinner/8008657/bug8008657.java fails #18054

prsadhuk · 2024-02-29T09:34:05Z

Test failed with the exception java.lang.IllegalArgumentException: (start <= value <= end) with no history of failing till date.
Investigation shows SpinnerDateModel constructor condition is not satisfied because
It seems it's a leap year problem that is being manifested..

As per test,
initDate Thu Feb 29 13:49:08 IST 2024 [ obtained by calendar.getTime();]
earliestDate Tue Feb 28 13:49:08 IST 2023 [obtained by calendar.add(Calendar.YEAR, -1);]
latestDate Wed Feb 28 13:49:08 IST 2024 [obtained by calendar.add(Calendar.YEAR, 1);]

Now, as per SpinnerDateModel constructor
earliestDate <= initDate condition is satisfied
but
latestDate >= initDate is not
so start <= value <= end condition fails

Not sure it anything can be done in Calendar class for this but fix is done in test taking leapyear into account so that we add a day if it's a leapyear when we consecutively do Calendar.add(YEAR, -1) and Calendar.add(YEAR,1) on leapyear date of 29Feb

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8327007: javax/swing/JSpinner/8008657/bug8008657.java fails (Bug - P3)

Reviewers

Phil Race (@prrace - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18054/head:pull/18054
$ git checkout pull/18054

Update a local copy of the PR:
$ git checkout pull/18054
$ git pull https://git.openjdk.org/jdk.git pull/18054/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18054

View PR using the GUI difftool:
$ git pr show -t 18054

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18054.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-02-29T09:34:36Z

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-02-29T09:34:55Z

@prsadhuk The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-02-29T09:39:51Z

Webrevs

01: Full - Incremental (20d888dd)
00: Full (e285b4c1)

shurymury · 2024-02-29T19:18:11Z

test/jdk/javax/swing/JSpinner/8008657/bug8008657.java

@@ -134,9 +134,20 @@ static String getOrientation(ComponentOrientation orientation) {
    static void createDateSpinner() {
        Calendar calendar = Calendar.getInstance();
        Date initDate = calendar.getTime();
+        int year = calendar.get(Calendar.YEAR);
+        boolean isLeapYear = ((year % 4 == 0) && (year % 100 != 0) || (year % 400 == 0));


Would it be better to use java.tim.Year.isLeap(long)?

shurymury · 2024-02-29T19:22:17Z

Can this test be changed to test multiple different dates, some of which are Feb 29th?

prrace · 2024-02-29T20:29:58Z

The problem comes down to this
% cat LeapDayBug.java
import java.util.Calendar;

public class LeapDayBug {
public static void main(String[] args) {
Calendar calendar = Calendar.getInstance();
calendar.set(2024, 1, 29);
System.out.println(calendar.getTime());
calendar.add(Calendar.YEAR, -1);
calendar.add(Calendar.YEAR, 1);
System.out.println(calendar.getTime());
}
}

% java LeapDayBug.java
Thu Feb 29 12:27:46 PST 2024
Wed Feb 28 12:27:46 PST 2024

So I'm not sure what the Calendar class could do about this since both
the -1 and the + 1 are correct considered standalone, but it just is not round trippable.
I filed a Calendar bug anyway so there's a record : https://bugs.openjdk.org/browse/JDK-8327088

prrace · 2024-02-29T20:34:03Z

Surely this just needs a one line fix

diff --git a/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java b/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
index cbeb0c185bb..49918278e34 100644
--- a/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
+++ b/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
@@ -137,6 +137,7 @@ static void createDateSpinner() {
         calendar.add(Calendar.YEAR, -1);
         Date earliestDate = calendar.getTime();
         calendar.add(Calendar.YEAR, 1);
+        calendar.add(Calendar.DAY_OF_MONTH, 1);
         Date latestDate = calendar.getTime();
         SpinnerModel dateModel = new SpinnerDateModel(initDate,
                 earliestDate,

prsadhuk · 2024-03-01T03:15:19Z

Surely this just needs a one line fix

diff --git a/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java b/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
index cbeb0c185bb..49918278e34 100644
--- a/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
+++ b/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
@@ -137,6 +137,7 @@ static void createDateSpinner() {
calendar.add(Calendar.YEAR, -1);
Date earliestDate = calendar.getTime();
calendar.add(Calendar.YEAR, 1);
   calendar.add(Calendar.DAY_OF_MONTH, 1);
   Date latestDate = calendar.getTime();
   SpinnerModel dateModel = new SpinnerDateModel(initDate,
           earliestDate,

Yes, I did thought about that but thought should we do it irrespectively of leap year or not, so atlast I thought of doing it for leapyear only so that it doesn't affect anyother scenarios...
But if you want to add 1 day to it irrespectively and cutshort the added code, I can do it too...

prsadhuk · 2024-03-04T05:16:35Z

Surely this just needs a one line fix

diff --git a/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java b/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
index cbeb0c185bb..49918278e34 100644
--- a/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
+++ b/test/jdk/javax/swing/JSpinner/8008657/bug8008657.java
@@ -137,6 +137,7 @@ static void createDateSpinner() {
calendar.add(Calendar.YEAR, -1);
Date earliestDate = calendar.getTime();
calendar.add(Calendar.YEAR, 1);
   calendar.add(Calendar.DAY_OF_MONTH, 1);
   Date latestDate = calendar.getTime();
   SpinnerModel dateModel = new SpinnerDateModel(initDate,
           earliestDate,

Done minimal change as suggested...

openjdk · 2024-03-04T18:59:21Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8327007: javax/swing/JSpinner/8008657/bug8008657.java fails

Reviewed-by: prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 46 new commits pushed to the master branch:

045eea1: 8327057: Parallel: Refactor ParMarkBitMap::iterate
f615ac4: 8309622: Re-examine the cache mechanism in BaseLocale
6f8d351: 8325725: Parallel: Refactor PSParallelCompact::fill_dense_prefix_end
8cfaceb: 8327125: SpinYield.report should report microseconds
59529a9: 8326688: Parallel: Remove unnecessary BOT update in UpdateOnlyClosure::do_addr
43c6f0b: 8326591: New test JmodExcludedFiles.java fails on Windows when --with-external-symbols-in-bundles=public is used
b5cd7ef: 8319901: Recursive lightweight locking: ppc64le implementation
0583f73: 8323183: ClassFile API performance improvements
b69d1b5: 8327042: G1: Parallelism used for redirty logged cards needs better control.
e889b46: 8327071: [Testbug] g-tests for cgroup leave files in /tmp on linux
... and 36 more: https://git.openjdk.org/jdk/compare/998d0baab0fd051c38d9fd6021628eb863b80554...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

prsadhuk · 2024-03-05T02:34:31Z

/integrate

openjdk · 2024-03-05T02:34:59Z

Going to push as commit b7540df.
Since your change was applied there have been 47 commits pushed to the master branch:

e1b661f: 8319900: Recursive lightweight locking: riscv64 implementation
045eea1: 8327057: Parallel: Refactor ParMarkBitMap::iterate
f615ac4: 8309622: Re-examine the cache mechanism in BaseLocale
6f8d351: 8325725: Parallel: Refactor PSParallelCompact::fill_dense_prefix_end
8cfaceb: 8327125: SpinYield.report should report microseconds
59529a9: 8326688: Parallel: Remove unnecessary BOT update in UpdateOnlyClosure::do_addr
43c6f0b: 8326591: New test JmodExcludedFiles.java fails on Windows when --with-external-symbols-in-bundles=public is used
b5cd7ef: 8319901: Recursive lightweight locking: ppc64le implementation
0583f73: 8323183: ClassFile API performance improvements
b69d1b5: 8327042: G1: Parallelism used for redirty logged cards needs better control.
... and 37 more: https://git.openjdk.org/jdk/compare/998d0baab0fd051c38d9fd6021628eb863b80554...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-03-05T02:35:04Z

@prsadhuk Pushed as commit b7540df.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8327007: javax/swing/JSpinner/8008657/bug8008657.java fails

e285b4c

openjdk bot added the rfr Pull request is ready for review label Feb 29, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Feb 29, 2024

shurymury reviewed Feb 29, 2024

View reviewed changes

Increment DAY_OF_MONTH to circumvent leapyear issue

20d888d

prrace approved these changes Mar 4, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 4, 2024

openjdk bot added the integrated Pull request has been integrated label Mar 5, 2024

openjdk bot closed this Mar 5, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 5, 2024

prsadhuk deleted the JDK-8327007 branch March 5, 2024 02:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8327007: javax/swing/JSpinner/8008657/bug8008657.java fails #18054

8327007: javax/swing/JSpinner/8008657/bug8008657.java fails #18054

prsadhuk commented Feb 29, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 29, 2024

openjdk bot commented Feb 29, 2024

mlbridge bot commented Feb 29, 2024 •

edited

Loading

shurymury Feb 29, 2024

shurymury commented Feb 29, 2024

prrace commented Feb 29, 2024 •

edited

Loading

prrace commented Feb 29, 2024 •

edited

Loading

prsadhuk commented Mar 1, 2024

prsadhuk commented Mar 4, 2024

openjdk bot commented Mar 4, 2024

prsadhuk commented Mar 5, 2024

openjdk bot commented Mar 5, 2024

openjdk bot commented Mar 5, 2024

8327007: javax/swing/JSpinner/8008657/bug8008657.java fails #18054

8327007: javax/swing/JSpinner/8008657/bug8008657.java fails #18054

Conversation

prsadhuk commented Feb 29, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Feb 29, 2024

openjdk bot commented Feb 29, 2024

mlbridge bot commented Feb 29, 2024 • edited Loading

Webrevs

shurymury Feb 29, 2024

Choose a reason for hiding this comment

shurymury commented Feb 29, 2024

prrace commented Feb 29, 2024 • edited Loading

prrace commented Feb 29, 2024 • edited Loading

prsadhuk commented Mar 1, 2024

prsadhuk commented Mar 4, 2024

openjdk bot commented Mar 4, 2024

prsadhuk commented Mar 5, 2024

openjdk bot commented Mar 5, 2024

openjdk bot commented Mar 5, 2024

prsadhuk commented Feb 29, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 29, 2024 •

edited

Loading

prrace commented Feb 29, 2024 •

edited

Loading

prrace commented Feb 29, 2024 •

edited

Loading