[rfc][icedtea-web] a new reproducer LiveConnect "set" tests
Adam Domurad
adomurad at redhat.com
Wed Oct 10 12:49:42 PDT 2012
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: recommentaton.java
Type: text/x-java
Size: 3108 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121010/e9b06ded/recommentaton.java
More information about the distro-pkg-dev
mailing list