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
7903118: Sanity Tests - Adding four JavaTest GUI newly automated sanity test scripts #24
Conversation
👋 Welcome back gollayadav! A progress list of the required criteria for merging this PR into |
@gollayadav This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
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 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 (@dbessono) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
Webrevs
|
|
||
import jthtest.ConfigTools; | ||
|
||
public class Config_SaveEdit extends ConfigTools { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this class ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking. I think the class 'Config_SaveEdit' is not required because I have extended 'ConfigTools' from the main test script and I have removed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, looks good
* at compile-time, but does not exist at | ||
* runtime. | ||
*/ | ||
@SuppressWarnings("deprecation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this done to suppress the warning about deprecated "createWorkDirInTemp" ? "createWorkDirInTemp" is used widely and it seems only in this pack of tests its usage is accompanied with '@SuppressWarnings("deprecation")'. Why is it deprecated? Should it be kept deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or possibly some other - non-deprecated method should be used instead of using a deprecated one and adding "@SuppressWarnings("deprecation")" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing. I will try to use the non-deprecated method and let you know.
* @throws ClassNotFoundException If the class does not find it in the | ||
* classpath. | ||
* | ||
* @throws InvocationTargetException Holds an exception thrown by an invoked | ||
* method or constructor. | ||
* | ||
* @throws NoSuchMethodException Occurs when a method is called that exists | ||
* at compile-time, but does not exist at | ||
* runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the idea of copying here a very general description of each exception? This comment seems applicable to all the tests in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing. I think this general description is not required for each exception and I have removed this for all the tests in PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the latest changes Test_Config_Load1.java and Test_Config_Save2.java look not compilable any more.
Sorry for the inconvenience caused. I have corrected the compilation errors now. Could you please check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks
/integrate |
@gollayadav |
/sponsor |
Going to push as commit a800429. |
@dbessono @gollayadav Pushed as commit a800429. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Adding below automated JavaTest GUI Sanity Test Scripts to the Jemmy regression suite and tested locally on three platforms(Linux, Windows, Mac OS) and working fine.
1.Test_Config_Edit3.java
2.Test_Config_Load1.java
3.Test_Config_New1.java
4. Test_Config_Save2.java
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jtharness pull/24/head:pull/24
$ git checkout pull/24
Update a local copy of the PR:
$ git checkout pull/24
$ git pull https://git.openjdk.java.net/jtharness pull/24/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 24
View PR using the GUI difftool:
$ git pr show -t 24
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jtharness/pull/24.diff