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

CODETOOLS-7902793: Fix IllegalMonitorStateException in JInternalFrameOperatorCloseTest #3

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

amresh-sahu
Copy link

@amresh-sahu amresh-sahu commented Nov 18, 2020

IllegalMonitorStateException has been fixed.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Referenced JBS issue must only be used for a single change
  • Change must be properly reviewed

Issue

  • CODETOOLS-7902793: Fix IllegalMonitorStateException in JInternalFrameOperatorCloseTest

Download

$ git fetch https://git.openjdk.java.net/jemmy-v2 pull/3/head:pull/3
$ git checkout pull/3

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Nov 18, 2020

👋 Welcome back amresh-sahu! 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.

@amresh-sahu
Copy link
Author

@amresh-sahu amresh-sahu commented Nov 18, 2020

please review @shurymury @akolarkunnu

@openjdk
Copy link

@openjdk openjdk bot commented Nov 18, 2020

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

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

7902793: Fix IllegalMonitorStateException in JInternalFrameOperatorCloseTest

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@shurymury) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added ready rfr labels Nov 18, 2020
@amresh-sahu
Copy link
Author

@amresh-sahu amresh-sahu commented Nov 18, 2020

Please review @akolarkunnu @shurymury

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 18, 2020

Webrevs

} catch (InterruptedException e1) {
e1.printStackTrace();
}
new QueueTool().waitEmpty();
Copy link
Collaborator

@shurymury shurymury Nov 18, 2020

Choose a reason for hiding this comment

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

There is a comment below, which explains what should be here:
// Making not to close the fame for 1 minute and expecting TimeoutExpiredException
// from waitClosed()

Which would mean that the lines in internalFrameClosing(InternalFrameEvent e) should just delay the execution for minute. The reason for doing that, it looks like, is that there is no other way to detect window closing but by the timeout.

Your fix is doing something completely different.

Copy link
Author

@amresh-sahu amresh-sahu Nov 24, 2020

Choose a reason for hiding this comment

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

Removed negative test scenario.
Currently, Testing that Internal frame is closing or not.

There were two scenarios one is happy path scenario where only closing internal frame was being tested and another was negative scenario where TimeoutExpiredException was expected so removed the negative test scenario.
@openjdk openjdk bot removed ready rfr labels Nov 24, 2020
Removed whitespace and tabs.
@openjdk openjdk bot added ready rfr labels Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready rfr
2 participants