[rfc][icedtea-web] a new reproducer LiveConnect "set" tests
Jana Fabrikova
jfabriko at redhat.com
Mon Oct 22 07:54:42 PDT 2012
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).
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),
Jana
On 10/10/2012 09:49 PM, Adam Domurad wrote:
> On 10/03/2012 01:29 PM, Jana Fabrikova wrote:
>> 2012-10-03 Jana Fabrikova <jfabriko at redhat.com>
>>
>> * /tests/reproducers/simple/JSToJSet:
>> adding a new reproducer for the second LiveConnect
>> test (Tests for setting members on Java side.)
>>
>>
>> I would like to ask for review of the attached patch,
>> thank you,
>> Jana
>
> Hi Jana, thanks for the test!
>
> I have some comments in-line with the test.
>
>
>> 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,104 @@
>> +<!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="JSToJ_auxiliary.js"></script>
>> + <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 type="text/javascript">
>> + document.getElementById('jstojSetApplet').setUpForSMTests();
>
> This calls the java applet to setup expected values, correct ? I would
> rather if the test passing did not rely on correct execution of
> JS-to-Java method calls - simply because a test should only fail if the
> behaviour it is testing is not working.
>
>> + var urlArgs = document.URL.split("?");
>> + var testid = urlArgs[1];
>> +
>> + if( testid != null ){
>> + document.getElementById( 'jstojSetApplet'
>> ).setStatusLabel(testid);
>> + appendMessageDiv(testid+"... ");
>> + }else{
>> + document.getElementById( 'jstojSetApplet'
>> ).setStatusLabel("url without ?");
>> + appendMessageDiv("no url arguments...");
>> + }
>> + switch(testid){
>> + case "int":
>> + test_set_int();
>> + break;
>> + case "double":
>> + test_set_double();
>> + break;
>> + case "float":
>> + test_set_float();
>> + break;
>> + case "long":
>> + test_set_long();
>> + break;
>> + case "boolean":
>> + test_set_boolean();
>> + break;
>> + case "char":
>> + test_set_char();
>> + break;
>> + case "byte":
>> + test_set_byte();
>> + break;
>> + case "intArrayElement":
>> + test_set_intArrayElement();
>> + break;
>> + case "intArrayBeyond":
>> + test_set_intArrayBeyond();
>> + break;
>> + case "regularString":
>> + test_set_regularString();
>> + break;
>> + case "specialCharsString":
>> + test_set_specialCharsString();
>> + break;
>> + case "null":
>> + test_set_null();
>> + break;
>> + case "Integer":
>> + test_set_Integer();
>> + break;
>> + case "Double":
>> + test_set_Double();
>> + break;
>> + case "Float":
>> + test_set_Float();
>> + break;
>> + case "Long":
>> + test_set_Long();
>> + break;
>> + case "Boolean":
>> + test_set_Boolean();
>> + break;
>> + case "Character":
>> + test_set_Character();
>> + break;
>> + case "Byte":
>> + test_set_Byte();
>> + break;
>> + case "DoubleArrayElement":
>> + test_set_DoubleArrayElement();
>> + break;
>> + case "DoubleFullArray":
>> + test_set_DoubleFullArray();
>> + break;
>> + default:
>> + appletStdOutLn('jstojSetApplet', "No argument in URL!
>> Should be e.g. JSToJSet.html?int");
>> + document.getElementById( 'jstojSetApplet'
>> ).setStatusLabel("Not a valid argument in URL! Should be e.g.
>> JSToJSet.html?int");
>> + break;
>> + }
>> + </script>
>> + </body>
>> +</html>
>> diff --git
>> a/tests/reproducers/simple/JSToJSet/resources/JSToJ_auxiliary.js
>> b/tests/reproducers/simple/JSToJSet/resources/JSToJ_auxiliary.js
>> new file mode 100644
>> --- /dev/null
>> +++ b/tests/reproducers/simple/JSToJSet/resources/JSToJ_auxiliary.js
>> @@ -0,0 +1,69 @@
>> +/*
>> +JSToJ_auxiliary.js
>> +This file contains auxiliary JavaScript functions for LiveConnect
>> tests output,
>> +the following reproducers have this file as a common resource:
>> +- JSToJGet
>> +- JSToJSet
>> +- JSToJFuncParam
>> +- JSToJFuncReturn
>> +- JSToJFuncResol
>> +- JSToJTypeConv
>> +- JToJSGet
>> +- JToJSSet
>> +- JToJSFuncParam
>> +- JToJSFuncReturn
>> +- JToJSEval
>> +*/
>> + [ ... file contents snipped ... ]
>
> I don't think this is the correct way to do this, although it can be
> good for tests to share common components.
>
> There will only be one copy in the deployment directory, because only
> one file can exist with a given file name. As well, if this file ever
> has to change, it will be difficult to tell why it isn't updating
> correctly (the reason would be that a test with the same-name file is
> overwriting it.)
>
> I think the correct way to do this is to have a reproducer folder that
> is named something like JSTestResources, and contains only this in the
> resources/ folder.
> Since all tests share the same deployment directory, it will be
> available to all tests. As well, it will only have to be altered in one
> place if it is ever altered.
>
>> +
>> 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,246 @@
>> +import java.applet.*;
>> +import java.awt.*;
>> +
>> +public class JSToJSet extends Applet {
>> +
>> + public String[] outputStrings = { "Test no. 1 - (int)",
>> + "Test no. 2 - (double)", "Test no. 3 - (float)",
>> + "Test no. 4 - (long)", "Test no. 5 - (boolean)",
>> + "Test no. 6 - (char)", "Test no. 7 - (byte)",
>> + "Test no. 8 - (int[] - element access)",
>> + "Test no. 9 - (int[] - beyond length)",
>> + "Test no.10 - (regular string)",
>> + "Test no.11 - (string with special characters)",
>> + "Test no.12 - (null)", "Test no.13 - (Integer)",
>> + "Test no.14 - (Double)", "Test no.15 - (Float)",
>> + "Test no.16 - (Long)", "Test no.17 - (Boolean)",
>> + "Test no.18 - (Character)", "Test no.19 - (Byte)",
>> + "Test no.20 - (Double[] - element access)",
>> + "Test no.21 - (Double[] - full array)" };
>> +
>> + public int i, iWanted;
>> + public double d, dWanted;
>> + public float f, fWanted;
>> + public long l, lWanted;
>> + public boolean b, bWanted;
>> + public char c, cWanted;
>> + public byte by, byWanted;
>> + public String rs, rsWanted;
>> + public String ss, ssWanted;
>> + public Object n, nWanted;
>> + public int[] ia = new int[5];
>> + public int[] iaWanted = new int[5];
>> +
>> + public Integer I, IWanted;
>> + public Double D, DWanted;
>> + public Float F, FWanted;
>> + public Long L, LWanted;
>> + public Boolean B, BWanted;
>> + public Character C, CWanted;
>> + public Byte By, ByWanted;
>> + public Double[] Da1 = new Double[10];
>> + public Double[] Da1Wanted = new Double[10];
>> + public Double[] Da2 = null;
>> + public Double[] Da2Wanted = null;
>> +
>> + public char[] ca = new char[3];
>> + public char[] caWanted = new char[3];
>> + public Character[] Ca = new Character[3];
>> + public Character[] CaWanted = new Character[3];
>
> That's a mouthful :)
>
> I have some recommentations for a way to refactor this test to be (in my
> opinion) a lot more easy to understand. I don't mean to make more work
> for you to do - but this test is quite hard to follow and would be hard
> to maintain.
>
> This idea uses eval + reflection - if you have questions, feel free to
> ask me :) (Either by email or IRC). Note that this does not require the
> 'auxiliary' JS file at all.
>
> If you'd bear with me ... (also attached as separate file)
>
> 1) Refactor JSToJSet.java as something simple, like:
> class ... {
> // members ...
> public int _int;
> public double _double;
> public float _float;
> public long _long;
> public boolean _boolean;
> public char _char;
> public byte _byte;
>
> public Integer _Integer;
> public Double _Double;
> public Float _Float;
> public Long _Long;
> public Boolean _Boolean;
> public Character _Character;
> public Byte _Byte;
>
> public String _String;
> public Object _Object;
>
> // Test 'size 1' arrays for simplicity, it is sufficient
> public int[] _intArray = new int[1];
> public char[] _charArray = new char[1];
>
> public Double[] _DoubleArray = new Double[1];
> public Character[] _CharacterArray = new Character[1];
>
> // checking method:
> // auxiliary methods for writing to stdout and stderr:
> public void printNewValueAndFinish(String fieldname) {
> Field field = getClass().getDeclaredField(fieldname);
> Object value = field.get(this);
> if (value != null && value.getClass().isArray()) {
> System.out.print("New array value is: " + Array.get(value,
> 0));
> } else {
> System.out.print("New value is: " + value);
> }
> System.out.print("*** APPLET FINISHED ***"); // Or whatever
> ending message you want to check for
>
> }
> }
>
>
>
> 2) Don't embed javascript in the html file, its hard to find there
>
> Instead, have something like this in JSToJava_Set.js:
> var urlArgs = document.URL.split("?");
> var testParams = urlArgs[1].split(";"); // You can choose a
> different character to separate the parmeters other than ';'
> var field = testParams[0], value = testParams[1];
> var applet = document.getElementById('jstojSetApplet')
>
> // Check for special value fields if we need to do anything that
> cannot be achieved by parsing the value as a javascript value
> if (value == << some special value here! >>){
> value = << do something like allocate a new array here >>;
> }
> eval('applet.' + field + ' = value');
>
> applet.printNewValueAndFinish(field)
>
> The label is nice but it complicates the test without adding to the
> purpose. I would be in favour of just getting rid of it.
>
>
>
>
> 3) In the test runner, use something like:
>
> @Test
> @TestInBrowsers(testIn = { Browsers.all })
> @NeedsDisplay
> public void AppletJSToJSet_int_Test() throws Exception {
> jsToJavaSetNormalTest("_int", "1"); // Passes '_int;0' by URL,
> checks for 'New value is: ...'
> }
>
> @Test
> @TestInBrowsers(testIn = { Browsers.all })
> @NeedsDisplay
> public void AppletJSToJSet_Double_Test() throws Exception {
> jsToJavaSetNormalTest("_Double", "1.0"); // Passes '_Double;0'
> by URL, checks for 'New value is: ...'
> }
>
>
> @Test
> @TestInBrowsers(testIn = { Browsers.all })
> @NeedsDisplay
> public void AppletJSToJSet_Double_Test() throws Exception {
> jsToJavaSetSpecialTest("_DoubleArray", <<some special value
> here>>,
> "New array value is: << something here >>"); // Passes
> '_Double;<<special value>>' by URL, checks for 'New array value is: ...'
> }
>
>
> Let me know if you have trouble following / implementing this!
> I hope it will result in a simplified test (especially use simple values
> - there is no need to test long strings of digits, or a string other
> than something obvious like "test", as well as one international string
> as you did).
>
>
> And as Jiri says, don't shoot me:)
>
> Cheers,
> - Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: adding_JSToJSet_reproducer_improved.patch
Type: text/x-patch
Size: 15314 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121022/5671114d/adding_JSToJSet_reproducer_improved.patch
More information about the distro-pkg-dev
mailing list