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

Mon Oct 22 09:32:52 PDT 2012

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