[rfc] [icedtea-web] Reproducer for clipboard (reopened PR708)
Jiri Vanek
jvanek at redhat.com
Thu Oct 25 04:58:09 PDT 2012
On 10/18/2012 06:30 PM, Adam Domurad wrote:
> On 05/29/2012 11:21 AM, Jiri Vanek wrote:
>> 2012-05-29 Jiri Vanek <jvanek at redhat.com>
>>
>> Added clipboard reproducers
>> * tests/jnlp_tests/signed/ClipboardContentSigned/resources/ClipboardContentSignedCopy1.jnlp:
>> Jnlp to invoke manual copying to clipboard on signed app, please note
>> the delayed death of application
>> * tests/jnlp_tests/signed/ClipboardContentSigned/resources/ClipboardContentSignedCopy2.jnlp:
>> Jnlp to invoke jtextfield like copying signed app, please note the
>> delayed death of application
>> * tests/jnlp_tests/signed/ClipboardContentSigned/resources/ClipboardContentSignedPaste1.jnlp:
>> Jnlp to invoke manual pasting on signed application
>> * tests/jnlp_tests/signed/ClipboardContentSigned/resources/ClipboardContentSignedPaste2.jnlp:
>> Jnlp to invoke jtextfield like pasting on signed application
>> * tests/jnlp_tests/signed/ClipboardContentSigned/srcs/ClipboardContentSigned.java:
>> Application which is trying to access clipboard by various ways.
>> * tests/jnlp_tests/signed/ClipboardContentSigned/testcases/ClipboardContentSignedTests.java:
>> Automated tests for four above jnlps.
>> * tests/jnlp_tests/simple/ClipboardContent/resources/ClipboardContentCopy1.jnlp:
>> Jnlp to invoke manual copying to clipboard on unsigned app, please note
>> the delayed death of application
>> * tests/jnlp_tests/simple/ClipboardContent/resources/ClipboardContentCopy2.jnlp:
>> Jnlp to invoke jtextfield like copying unsigned app, please note the
>> delayed death of application
>> * tests/jnlp_tests/simple/ClipboardContent/resources/ClipboardContentPaste1.jnlp:
>> Jnlp to invoke manual pasting on unsigned application
>> * tests/jnlp_tests/simple/ClipboardContent/resources/ClipboardContentPaste2.jnlp:
>> Jnlp to invoke jtextfield like pasting on unsigned application
>> * tests/jnlp_tests/simple/ClipboardContent/srcs/ClipboardContent.java:
>> Application which is trying to access clipboard by various ways.
>> * tests/jnlp_tests/simple/ClipboardContent/testcases/ClipboardContentTests.java:
>> Automated tests for first and third of above four jnlps. The tests of
>> second and fourth is disabled due to necessary manual interaction
>>
>> Hi all ! whatever happens in PR708 this reproducers should remains green ;)
>>
>> There is one part of in testcases which may be unclear to reviewer, and it is parallel run of
>> copier (test/application) and paster (application/test). In way that test have copied to clipboard
>> and javaws have paste from it, then there is no issue, because test is eternal running clipboard
>> owner (who is waiting for his javaws to finish) so javaws can paste without problems.
>> The problem occurred when javaws copied to clipboard and test have to read from it. In this case
>> I had to make javaws run in parallel with test for a while (I have 10seconds timeout, and test is
>> NOT waiting for it to time-out, I just neede to have clipboard owner (javaws)alive, until I test
>> the value. The issue is that when (java as) clipboard owner ends, everything it puts to clipboard
>> die with it.
>>
>> I was on doubt just to test access to clipboard and so get rid of this paralelisation but it did
>> not seems to me enough. I would rather test the real value in clipboard.
>>
>>
>> So rfc! Best regards
>> J.
>>
>
> Hi Jiri, I remember looking at this a while back, sorry for letting it sit for so long (although you
> should have pinged it meanwhile :).
>
> Thanks for tackling this, comments inline.
>
>> diff -r 526df16b6e27
>> tests/jnlp_tests/signed/ClipboardContentSigned/resources/ClipboardContentSignedCopy1.jnlp
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/tests/jnlp_tests/signed/ClipboardContentSigned/resources/ClipboardContentSignedCopy1.jnlp
>> Tue May 29 17:20:14 2012 +0200
>> @@ -0,0 +1,58 @@
>> +<!--
>> +
...
>> + }
>> +
>> + private void printFlavors() {
>> +//jsut for debuging
>
> jsut -> just, debuging -> debugging.
>
fixed
>> +// Toolkit toolkit = Toolkit.getDefaultToolkit();
>> + private static final List<String> l = Collections.unmodifiableList(Arrays.asList(new
>> String[]{"-Xtrustall"}));
>
> I don't like this 'l' being used to describe this list (yes, I know that its everywhere :D), but, do
> as you will.
>
intentionally overlooked O:)
(means fixed)
>> +
>> + public static void putToClipboard(String str) {
>> + Toolkit toolkit = Toolkit.getDefaultToolkit();
>> + Clipboard clipboard = toolkit.getSystemClipboard();
>> + StringSelection strSel = new StringSelection(str);
>> + clipboard.setContents(strSel, null);
>> + }
>> +
>> + public static String pasteFromClipboard() throws UnsupportedFlavorException, IOException {
>> + Toolkit toolkit = Toolkit.getDefaultToolkit();
>> + Clipboard clipboard = toolkit.getSystemClipboard();
>> + Transferable clipData = clipboard.getContents(clipboard);
>> + String s = (String) (clipData.getTransferData(
>> + DataFlavor.stringFlavor));
>> + return s;
>> + }
>> +
>> + @Test
>> + public void assertClipboardIsWorking() throws Exception {
>> + putToClipboard(emptyContent);
>> + Assert.assertEquals("Clipboard must contains new value, did not", emptyContent,
>> pasteFromClipboard());
>> + putToClipboard(contentC);
>> + Assert.assertEquals("Clipboard must contains new value, did not", contentC,
>> pasteFromClipboard());
>> + }
>> +
>> + private static class AsyncJavaws implements Runnable {
>
> The copypasting of the helper classes between ClipboardContentTest and ClipboardContentSignedTest is
> pretty hard for me to accept ... Do you think these are general enough to be pushed into 'test
hmmm The ones used in test can be placed into some utils in tests-extensions. Just see how it looks
now. If you agree that all three separated classes are general enough, then this can work.
> extensions' ? It would also keep intent of the tests clear. I'd be happy if you had other solutions.
> Perhaps you should have only one testing class, and have the signed/unsigned components be
> 'resources'? (Custom reproducer could handle this on one test, but its probably better to have
I was thinking about this.But at the end I have decided rather to have some duplicate code then to
mingled with dependences for reproducer. In this case it snot because I'm lazy, but because I prefer
those independent reproducer even before not-duplicate code (it is 99% really simple duplication)
> custom reproducers always be 'specially prepared' tests)
>
>> +
>> + private final boolean headless;
>> + private final String url;
>> + private ProcessResult result;
>> + private ContentReaderListener contentReaderListener;
>> + private ContentReaderListener errorReaderListener;
>> +
>> + public AsyncJavaws(String url, boolean headless, ContentReaderListener
>> contentReaderListener, ContentReaderListener errorReaderListener) {
>> + this.url = url;
>> + this.headless = headless;
>> + this.contentReaderListener = contentReaderListener;
>> + this.errorReaderListener = errorReaderListener;
>> + }
>> +
>> + @Override
>> + public void run() {
>> + try {
>> + if (headless) {
>> + result = server.executeJavawsHeadless(l, url, contentReaderListener,
>> errorReaderListener);
>
> This method signature doesn't exist now, can you either add 'null' to the end, or add a new delegate
> with the patch ?
sure. Fixed
>
>> + } else {
...
>> + ServerAccess.logOutputReprint("Starting tread with " + url + " and waiting for result
>> or string " + waitingFor);
>
> tread -> thread
>
>> + ServerAccess.logOutputReprint("Waiting done. Ressult have been delivered");
>
> Ressult -> result
>
>> + }
>> + if (!canRun) {
>> + ServerAccess.logOutputReprint("Waiting done. String " + waitingFor + " delivered");
>> + }
>> + }
>> +
>> + public String getErr() {
>> + return err.toString();
>> + }
>> +
>> + public String getOutput() {
>> + return output.toString();
>> + }
>> +
>> + public AsyncJavaws getAj() {
>
> I prefer the more verbose getAsyncJavaws() (if nothing else, Java coding is great typing practice
> :), especially if it becomes a general test-extension. If it stays within this test file, I can live
> with it.
I extracted it too. Se above.
>
>> + return aj;
>> + }
>> + }
>> +
>> + @Test
>> + @Bug(id = "PR708")
>> + public void ClipboardContentSignedTestCopy1() throws Exception {
>> + putToClipboard(emptyContent);
>> + Assert.assertEquals("Clipboard must contains new value, did not", emptyContent,
>> pasteFromClipboard());
>> + WaitingForStringProcess wfsp = new
>> WaitingForStringProcess("/ClipboardContentSignedCopy1.jnlp", true, "copied");
>> + wfsp.run();
>> + String ss = pasteFromClipboard();
>> + Assert.assertEquals("Clipboard must contains new value, did not", contentC, ss);
>> + String s = "xception";
>
> I'm assuming no problems exist for javaws and matching 'xception'. Can you extract 's' as a
> meaningfully named constant ? Eg, XCEPTION would do fine.
ok,
>
>> + Assert.assertFalse("ClipboardContentSignedCopy stderr should not contains " + s + " but did ",
>> wfsp.getErr().contains(s));
...
>> + putToClipboard(emptyContent);
>> + Assert.assertEquals("Clipboard must contains new value, did not", emptyContent,
>> pasteFromClipboard());
>
> contains -> contain
Fixed
>
>> + //now put the tested data
>> + putToClipboard(contentP);
>> + Assert.assertEquals("Clipboard must contains new value, did not", contentP,
>> pasteFromClipboard());
>> + Assert.assertEquals("Clipboard must contains new value, did not", contentP,
>> pasteFromClipboard());
>> + ServerAccess.ProcessResult pr = server.executeJavaws(l,
>> "/ClipboardContentSignedPaste2.jnlp");
>> + Assert.assertTrue("ClipboardContentSignedTestPaste stdout should contains " + contentP +
>> " but didn't", pr.stdout.contains(contentP));
>> + String s = "xception";
>> + Assert.assertFalse("ClipboardContentSignedTestPaste stderr shouldnot contains " + s + "
>> but did ", pr.stderr.contains(s));
>
> shouldnot contains -> should not contain
fixed
>
>> + Assert.assertFalse(pr.wasTerminated);
>> +
>> + public void proceeed(String arg) throws Exception {
>
> proceeed -> proceed ? Unless you have a reason for three e's :)
>
fixed:)
>> + proceeed(arg, 0);
>> + }
>> +
...n strSel = new StringSelection(str);
>> + clipboard.setContents(strSel, null);
>> + }
>
> Its not an issue at all, but the 'public' is a little funny for something that has no external
> existence :)
hmhm left public ook?
>
>> +
>> + public static String pasteFromClipboard() throws UnsupportedFlavorException, IOException {
>> + Assert.assertEquals("Clipboard content must not be changed, was, did not", emptyContent,
>> ss);
>
> Should this be 'must not be changed, but was' ?
fixed, thanx
>
>> + Assert.assertNotNull("Result had to be delivered, was not", wfsp.getAj().getResult());
>> + String s = "xception";
>> + Assert.assertTrue("ClipboardContentSignedCopy stderr should contains " + s + " but did ",
>> wfsp.getAj().getResult().stderr.contains(s));
>
> 'but did' -> 'but did not'.
>
oook
>> + }
>> +
>> + //@Test needs awt robot to close dialog
>
> Surely simply adding HEADLESS will stop this dialog from occurring (and thus needing to be closed) ?
>
>> + @Bug(id = "PR708")
>> + @NeedsDisplay
>> + public void ClipboardContentTestCopy2() throws Exception {
>> + putToClipboard(emptyContent);
>> + Assert.assertEquals("Clipboard must contains new value, did not", emptyContent,
>> pasteFromClipboard());
>> + WaitingForStringProcess wfsp = new WaitingForStringProcess("/ClipboardContentCopy2.jnlp",
>> false, "copied");
>> + wfsp.run();
>> + String ss = pasteFromClipboard();
>> + Assert.assertEquals("Clipboard content must not be changed, was", emptyContent, ss);
>> + Assert.assertNotNull("Result had to be delivered, was not", wfsp.getAj().getResult());
>> + String s = "xception";
>> + Assert.assertTrue("ClipboardContentSignedCopy stderr should contains " + s + " but did ",
>> wfsp.getAj().getResult().stderr.contains(s));
>
> should contains -> should contain
> but did -> but did not
fixed
>
>> +
>> + }
>> +
>> + @Test
>> + @Bug(id = "PR708")
>> + public void ClipboardContentTestPaste1() throws Exception {
>> + //necessery errasing
>> + putToClipboard(emptyContent);
>> + Assert.assertEquals("Clipboard must contains new value, did not", emptyContent,
>> pasteFromClipboard());
>
> contains -> contain
done
>
>> + //now put the tested data
>> + putToClipboard(contentP);
>> + Assert.assertEquals("Clipboard must contains new value, did not", contentP,
>> pasteFromClipboard());
>> + ServerAccess.ProcessResult pr = server.executeJavawsHeadless(l,
>> "/ClipboardContentPaste1.jnlp");
>> + Assert.assertFalse("ClipboardContentTestPaste stdout should not contains " + contentP + "
>> but didn", pr.stdout.contains(contentP));
>
> didn -> didn't
>
>> + String s = "xception";
>> + Assert.assertTrue("ClipboardContentTestPaste stderr should contains " + s + " but didn't
>> ", pr.stderr.contains(s));
>> + Assert.assertFalse(pr.wasTerminated);
>> + Assert.assertEquals((Integer) 0, pr.returnValue);
>> + }
>> +
>> + //@Test needs awt robot to close dialog
>
> Same as above, seems to me making the run headless will suffice to have this as an automated test.
no. Headless tests are present. When you wil go through swing clipboard access, you will see there
is slight difference. This "test: is checking this.So this can not be headless. I think it does not
meter, so I left it here for record. Comment added to code
>
>> + @Bug(id = "PR708")
...
>> + }
>> +}
>
> Content-wise the patch is good, nice work handling this tricky-to-test feature!
> Apologies for the nitpicking :)
>
I'm Glad for them!
J
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ClipBoardReproducer3.diff
Type: text/x-patch
Size: 61079 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121025/19733672/ClipBoardReproducer3.diff
More information about the distro-pkg-dev
mailing list