[rfc][icedtea-web] a new reproducer LiveConnect "set" tests

Wed Oct 24 07:39:28 PDT 2012

Hello Adam, thanks again. The reproducer seems much simpler already:-)

I incorporated the comments in new version of the patch (the attached 
file).

- the initializations of variables really could be simplified, so i did it.
- the AssertFalse(has there been an error or exception) was not included 
, since all the tests pass in this version, maybe if there is some 
typical error/exception in the future use, it would make sense,

Jana

On 10/22/2012 06:32 PM, Adam Domurad wrote:
> On 10/22/2012 10:54 AM, Jana Fabrikova wrote:
>> Hello Adam,
>>
>> thank you for the comments. I modified the JSToJSet reproducer
>> according to your suggestions, please see the attached patch (new
>> version of the JSToJSet reproducer).
>
> Thank you very much for the update! I don't mean to torture you with
> rewrites, but I do find this a lot more tasteful :).
>
> Overall looks a lot better, small comments inline.
>
>>
>> The behaviour when using eval and reflection is different in 2 ways:
>> - the testcase arrayBeyondLength was meaningless from the Java side,
>> so it is not included
>> - all the testcases look only at the string output, so the values are
>> tested, but the types are not (I guess this is not a problem, since
>> the variables in Java have its defined types and a problem will occur
>> if JS tries to set a variable to a value of a bad type anyway),
>
> Sounds fine to me.
>
>>
>> Jana
>>
>
>> diff --git a/adding_JSToJSet_reproducer_improved.patch
>> b/adding_JSToJSet_reproducer_improved.patch
>> new file mode 100644
>> diff --git a/tests/reproducers/simple/JSToJSet/resources/JSToJSet.html
>> b/tests/reproducers/simple/JSToJSet/resources/JSToJSet.html
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/reproducers/simple/JSToJSet/resources/JSToJSet.html
>> @@ -0,0 +1,25 @@
>> +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
>> +<html lang="en-US">
>> + <head>
>> + <title>JavaScript to Java LiveConnect - Set values from applet</title>
>> + <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
>> +
>> + <script language="JavaScript" src="JSToJava_Set.js"></script>
>> +
>> + </head>
>> + <body>
>> +
>> + <h2> The JSToJSet html page</h2>
>> +
>> + <applet code="JSToJSet" width="1000" height="100"
>> id="jstojSetApplet" MAYSCRIPT>
>> + <param name="jnlp_href" value="jstoj-set.jnlp">
>> + </applet>
>> +
>> + <div id="messageDiv"></div>
>> +
>> + <script language="javascript">
>> + doSetTests();
>> + </script>
>> +
>> + </body>
>> +</html>
>> diff --git
>> a/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> b/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> @@ -0,0 +1,39 @@
>> +function doSetTests( ){
>> +
>> + var urlArgs = document.URL.split("?");
>> + var testParams = urlArgs[1].split(";");
>> + var applet = document.getElementById('jstojSetApplet');
>> + var field = testParams[0];
>> + var value = testParams[1];
>> +
>> + if( value === "JavaScript"){
>> + if( field === "_char"){
>> + value = 97;
>> + }
>> +
>> + if( field === "_Character"){
>> + value = new (applet.Packages).java.lang.Character(65);
>> + }
>> +
>> + if( field === "_specialString"){
>> + value = "��〒£$ǣ€��";
>> + }
>> + }else if(value.indexOf('[') != -1){
>> +
>> + var elem = value.substring(1);
>> + value = new Array();
>> + eval('value[0] = elem');
>> +
>> + }
>> +
>> + eval('applet.' + field + '= value');
>> +
>> + //modifiing _intArray[0] into _intArray
>> + // _DoubleArray[0] into _DoubleArray
>> + var nameEnd = field.indexOf('[');
>> + if( nameEnd != -1){
>> + field = field.substring(0,nameEnd);
>> + }
>> +
>> + applet.printNewValueAndFinish(field);
>> +}
>> diff --git
>> a/tests/reproducers/simple/JSToJSet/resources/jstoj-set.jnlp
>> b/tests/reproducers/simple/JSToJSet/resources/jstoj-set.jnlp
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/reproducers/simple/JSToJSet/resources/jstoj-set.jnlp
>> @@ -0,0 +1,21 @@
>> +<?xml version="1.0" encoding="UTF-8"?>
>> +<jnlp spec="1.0+" codebase="" href="jstoj-set.jnlp">
>> + <information>
>> + <title>JavaScript to Java LiveConnect - Set</title>
>> + <vendor>RedHat</vendor>
>> + <homepage
>> href="http://icedtea.classpath.org/wiki/IcedTea-Web#Testing_IcedTea-Web"/>
>>
>> + <description>LiveConnect - tests for setting members on Java
>> side.</description>
>> + </information>
>> + <resources>
>> + <!-- Application Resources -->
>> + <j2se version="1.6+"
>> + href="http://java.sun.com/products/autodl/j2se"/>
>> + <jar href="JSToJSet.jar" main="true" />
>> + </resources>
>> + <applet-desc
>> + name="JS to J Set"
>> + main-class="JSToJSet"
>> + width="1000"
>> + height="100">
>> + </applet-desc>
>> +</jnlp>
>> diff --git a/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> b/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> @@ -0,0 +1,73 @@
>> +import java.applet.*;
>> +import java.awt.*;
>> +import java.lang.reflect.Array;
>> +import java.lang.reflect.Field;
>> +
>> +public class JSToJSet extends Applet {
>> +
>> + public int _int;
>> + public double _double;
>> + public float _float;
>> + public long _long;
>> + public boolean _boolean;
>> + public char _char;
>> + public byte _byte;
>> + public String _String;
>> + public String _specialString;
>> + public Object _Object;
>> + public int[] _intArray = new int[1];
>> + public Integer _Integer;
>> + public Double _Double;
>> + public Float _Float;
>> + public Long _Long;
>> + public Boolean _Boolean;
>> + public Character _Character;
>> + public Byte _Byte;
>> + public Double[] _DoubleArray = new Double[10];
>> + public Double[] _DoubleArray2 = null;
>> + public char[] _charArray = new char[1];
>> + public Character[] _CharacterArray = new Character[1];
>> +
>> + public void init() {
>> + String initStr = "JSToJSet applet initialized.";
>> + System.out.println(initStr);
>> +
>> + // setting up variables that will be accessed by JS
>> + _int = 0;
>> + _double = 0.0;
>> + _float = 0F;
>> + _long = 0L;
>> + _boolean = false;
>> + _char = 'A';
>> + _byte = 0;
>
> Can you remove all the assignments that are already default for members
> ( I may be missing a reason why not, in which case do say so)?
> (In Java all numbers default to 0, booleans default to false, objects to
> null.)
>
>> + _String = "";
>> + _specialString = "";
>> + _Object = new String("non-null object");
>
> I think it'd be clearer and shorter if the _Object assignment was in the
> _Object definition.
>
>> + _Integer = 0;
>> + _Double = 0.0;
>> + _Float = 0F;
>> + _Long = 0L;
>> + _Boolean = false;
>> + _Character = 'B';
>
> Similarly, anything that isn't default can be moved to the definition.
>
>> + _Byte = null;
>> +
>> + _intArray[0] = 0;
>> + _DoubleArray[0] = 0.0;
>
> The 0s are already default contents and not needed (unless theres a need
> for the assignment that I'm missing).
>
>> +
>> + String setupStr = "JSToJSet applet set up for SET tests.";
>> + System.out.println(setupStr);
>> +
>> + }
>> +
>> + public void printNewValueAndFinish(String fieldname) throws Exception {
>> + Field field = getClass().getDeclaredField(fieldname);
>> + Object value = field.get(this);
>> + if (value != null && value.getClass().isArray()) {
>> + System.out.println("New array value is: " + Array.get(value, 0));
>> + } else {
>> + System.out.println("New value is: " + value);
>> + }
>> + System.out.println("afterTests");
>> + }
>> +
>> +}
>> diff --git
>> a/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> b/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> @@ -0,0 +1,258 @@
>> +/* JSToJSetTest.java
>> +Copyright (C) 2012 Red Hat, Inc.
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or
>> +modify it under the terms of the GNU General Public License as
>> published by
>> +the Free Software Foundation, version 2.
>> +
>> +IcedTea is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to
>> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> + */
>> +
>> +import net.sourceforge.jnlp.ProcessResult;
>> +import net.sourceforge.jnlp.ServerAccess;
>> +import net.sourceforge.jnlp.browsertesting.BrowserTest;
>> +import net.sourceforge.jnlp.browsertesting.Browsers;
>> +import net.sourceforge.jnlp.closinglisteners.CountingClosingListener;
>> +import net.sourceforge.jnlp.annotations.NeedsDisplay;
>> +import net.sourceforge.jnlp.annotations.TestInBrowsers;
>> +import org.junit.Assert;
>> +
>> +import org.junit.Test;
>> +
>> +public class JSToJSetTest extends BrowserTest {
>> +
>> + private final String exceptionStr = "xception";
>> + private final String errorStr = "rror";
>> + private final String initStr = "JSToJSet applet initialized.";
>> + private final String setupStr = "JSToJSet applet set up for SET
>> tests.";
>> + private final String afterStr = "afterTests";
>> +
>> + private class CountingClosingListenerImpl extends
>> CountingClosingListener {
>> +
>> + @Override
>> + protected boolean isAlowedToFinish(String s) {
>> + if (s.contains(exceptionStr) || s.contains(errorStr)) {
>> + return true;
>> + }
>> + return (s.contains(initStr) && s.contains(setupStr) &&
>> s.contains(afterStr));
>> + }
>> + }
>> +
>> + private void evaluateStdoutContents(String expectedStdout,
>> ProcessResult pr) {
>> + // Assert that the applet was initialized.
>> + Assert.assertTrue("JSToJSetTest stdout should contain \"" + initStr
>> + + "\" but it didn't.", pr.stdout.contains(initStr));
>> +
>> + // Assert that the applet was set up for the SM tests.
>> + Assert.assertTrue("JSToJSetTest stdout should contain \"" + setupStr
>> + + "\" but it didn't.", pr.stdout.contains(setupStr));
>> +
>> + // Assert that there were no errors / exceptions
>> + Assert.assertFalse("There were problems: "+pr.stdout,
>> + pr.stdout.contains(errorStr) || pr.stdout.contains(exceptionStr) );
>
> Unless I'm missing something here, this should be checking stderr
> (uncaught exceptions always print in standard error).
>
> Another issue here though is that the applet can randomly print
> exceptions when the applet window is closed.
> This will cause the tests to erroneously come up as failures. I would
> recommend checking one or more specific exceptions that are likely to
> occur, and generally assume that the test will fail on the next
> 'assertTrue' if an exception interrupts normal execution.
>
>> +
>> + // Assert that the values set by JavaScript are ok
>> + Assert.assertTrue("The output should be: "+expectedStdout+", but is:
>> "+pr.stdout,
>> + pr.stdout.contains(expectedStdout));
>> +
>> + }
>> +
>> + private void jsToJavaSetNormalTest(String fieldStr, String valueStr)
>> throws Exception {
>> + String strURL = "/JSToJSet.html?" + fieldStr + ";" + valueStr;
>> + ProcessResult pr = server.executeBrowser(strURL, new
>> CountingClosingListenerImpl(), new CountingClosingListenerImpl());
>> + String expectedStdout = "New value is: " + valueStr;
>> + evaluateStdoutContents(expectedStdout, pr);
>> + }
>> +
>> + private void jsToJavaSetSpecialTest(String fieldStr, String
>> valueStr, int testType) throws Exception {
>> + String strURL = "/JSToJSet.html?";
>> + String expectedStdout = "";
>> + switch( testType ){
>> + case 0://array element
>> + strURL += fieldStr + ";" + valueStr;
>> + expectedStdout = "New array value is: "+valueStr;
>> + break;
>> + case 1://whole array, set 1st element
>> + strURL += fieldStr + ";[" + valueStr;
>> + expectedStdout = "New array value is: "+valueStr;
>> + break;
>> + case 2://char et al - to be set at JS side
>> + strURL += fieldStr + ";JavaScript";
>> + expectedStdout = "New value is: "+valueStr;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + ProcessResult pr = server.executeBrowser(strURL, new
>> CountingClosingListenerImpl(), new CountingClosingListenerImpl());
>> + evaluateStdoutContents(expectedStdout, pr);
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_int_Test() throws Exception {
>> + jsToJavaSetNormalTest("_int", "1");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_double_Test() throws Exception {
>> + jsToJavaSetNormalTest("_double", "1.0");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_float_Test() throws Exception {
>> + jsToJavaSetNormalTest("_float", "1.1");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_long_Test() throws Exception {
>> + jsToJavaSetNormalTest("_long", "10000");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_boolean_Test() throws Exception {
>> + jsToJavaSetNormalTest("_boolean", "true");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_char_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_char", "a", 2);
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_byte_Test() throws Exception {
>> + jsToJavaSetNormalTest("_byte", "10");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_intArrayElement_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_intArray[0]", "1", 0);
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_regularString_Test() throws Exception {
>> + jsToJavaSetNormalTest("_String", "teststring");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_specialCharsString_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_specialString", "��〒£$ǣ€��", 2);
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_null_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Object", "null");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Integer_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Integer", "1");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Double_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Double", "1.0");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Float_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Float", "1.1");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Long_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Long", "10000");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Boolean_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Boolean", "true");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Character_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_Character", "A", 2);
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_Byte_Test() throws Exception {
>> + jsToJavaSetNormalTest("_Byte", "100");
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_DoubleArrayElement_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_DoubleArray[0]", "1.1", 0);
>> + }
>> +
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_DoubleFullArray_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_DoubleArray", "0.1", 1);
>> + }
>> +
>> +}
>
> Good work! Really. It's structured a lot more like a test should be now
> (with the testcases/ test class responsible for determining if a test
> passes or fails).
> Very close to push-ready.
>
> Cheers,
> - Adam
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: adding_JSToJSet_reproducer_2ndcorrection.patch
Type: text/x-patch
Size: 14035 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121024/22f87962/adding_JSToJSet_reproducer_2ndcorrection.patch 