[rfc][icedtea-web] additions to test-extensions needed for the new AWTFramework
Jiri Vanek
jvanek at redhat.com
Thu Dec 13 02:21:20 PST 2012
On 12/13/2012 11:03 AM, Jana Fabrikova wrote:
> 2012-12-13 Jana Fabrikova <jfabriko at redhat.com>
>
> * /tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java:
> Added several new versions of method executeBrowser with lists
> of ContentReaderListeners as arguments.
It is common to use ..ew versions of (executeBrowser) with lis... parentheses which measn
method/target...
>
> * /tests/test-extensions/net/sourceforge/jnlp/ProcessWrapper.java:
> Added new versions of the constructor and methods for adding
> ContentReaderListeners using List<ContentReaderListener>
> instead of one ContentReaderListener.
> Added a new version of constructor with less arguments (used
> in case when several arguments would be passed as null, thus
> causing ambiguity).
same
>
> * /tests/reproducers/simple/SingeInstanceServiceTest/testcases/SingleInstanceTest.java:
> Modified the call of executeBrowser method with null arguments
> into a call of new method without the null arguments
> (getting rid of an ambiguous call).
same
>
>
> I would like to ask for review of the attached patch,
> thank you,
> Jana
>
> AWTFramework_preparations.patch
>
>
> diff --git a/tests/reproducers/simple/SingleInstanceServiceTest/testcases/SingleInstanceTest.java b/tests/reproducers/simple/SingleInstanceServiceTest/testcases/SingleInstanceTest.java
> --- a/tests/reproducers/simple/SingleInstanceServiceTest/testcases/SingleInstanceTest.java
> +++ b/tests/reproducers/simple/SingleInstanceServiceTest/testcases/SingleInstanceTest.java
> @@ -109,7 +109,7 @@
> try {
> boolean isJavawsTest = isJnlp(launchFile);
> pr = isJavawsTest ? server.executeJavawsHeadless(launchFile, null, null)
> - : server.executeBrowser(launchFile, null, null);
> + : server.executeBrowser(launchFile);
> } catch (Exception ex) {
> ServerAccess.logException(ex);
> } finally {
> diff --git a/tests/test-extensions/net/sourceforge/jnlp/ProcessWrapper.java b/tests/test-extensions/net/sourceforge/jnlp/ProcessWrapper.java
> --- a/tests/test-extensions/net/sourceforge/jnlp/ProcessWrapper.java
> +++ b/tests/test-extensions/net/sourceforge/jnlp/ProcessWrapper.java
> @@ -64,6 +64,20 @@
> public ProcessWrapper() {
> }
>
> + public ProcessWrapper(String toBeExecuted, List<String> otherargs, URL u){
> + Assert.assertNotNull(u);
> + Assert.assertNotNull(toBeExecuted);
> + Assert.assertTrue(toBeExecuted.trim().length() > 1);
> + if (otherargs == null) {
> + otherargs = new ArrayList(1);
> + }
> + List<String> urledArgs = new ArrayList(otherargs);
> + urledArgs.add(0, toBeExecuted);
> + urledArgs.add(u.toString());
> + this.args = urledArgs;
> + this.vars=null;
> + }
> +
This smell with doubled code. Can you extract all the duplicate parts?
> public ProcessWrapper(String toBeExecuted, List<String> otherargs, URL u, ContentReaderListener stdoutl, ContentReaderListener stderrl, String[] vars) throws Exception {
> Assert.assertNotNull(u);
> Assert.assertNotNull(toBeExecuted);
> @@ -81,6 +95,22 @@
>
> }
>
> + public ProcessWrapper(String toBeExecuted, List<String> otherargs, URL u, List<ContentReaderListener> stdoutl, List<ContentReaderListener> stderrl, String[] vars) throws Exception {
> + Assert.assertNotNull(u);
> + Assert.assertNotNull(toBeExecuted);
> + Assert.assertTrue(toBeExecuted.trim().length() > 1);
> + if (otherargs == null) {
> + otherargs = new ArrayList(1);
> + }
> + List<String> urledArgs = new ArrayList(otherargs);
> + urledArgs.add(0, toBeExecuted);
> + urledArgs.add(u.toString());
> + this.args = urledArgs;
> + this.addStdOutListeners(stdoutl);
> + this.addStdErrListeners(stderrl);
> + this.vars=vars;
> + }
This smells with similar duplication.
> +
> ProcessWrapper(final List<String> args, File dir, ContentReaderListener stdoutl, ContentReaderListener stderrl, String[] vars) {
> this.args = args;
> this.dir = dir;
> @@ -89,6 +119,14 @@
> this.vars = vars;
> }
>
> + public ProcessWrapper(final List<String> args, File dir, List<ContentReaderListener> stdoutl, List<ContentReaderListener> stderrl, String[] vars) {
> + this.args = args;
> + this.dir = dir;
> + this.addStdOutListeners(stdoutl);
> + this.addStdErrListeners(stderrl);
> + this.vars = vars;
> + }
> +
> public final void addStdOutListener(ContentReaderListener l) {
> if (l == null) {
> return;
> @@ -105,6 +143,22 @@
>
> }
>
> + public final void addStdOutListeners(List<ContentReaderListener> l) {
> + if (l == null) {
> + return;
> + }
> + stdoutl.addAll(l);
> +
> + }
> +
> + public final void addStdErrListeners(List<ContentReaderListener> l) {
> + if (l == null) {
> + return;
> + }
> + stderrl.addAll(l);
> +
> + }
> +
> /**
> * @return the args
> */
> diff --git a/tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java b/tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java
> --- a/tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java
> +++ b/tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java
> @@ -573,6 +573,10 @@
> return executeBrowser(getBrowserParams(), resource, stdoutl, stderrl);
> }
>
> + public ProcessResult executeBrowser(String resource, List<ContentReaderListener> stdoutl, List<ContentReaderListener> stderrl) throws Exception {
> + return executeBrowser(getBrowserParams(), resource, stdoutl, stderrl);
> + }
> +
> /**
> * wrapping method to executeProcess (eg: javaws arg arg http://localhost:port/resource)
> * will execute default javaws (@see JAVAWS_BUILD_BIN) upon default url upon cached server (@see SERVER_NAME @see getPort(), @see getInstance()))
> @@ -592,7 +596,7 @@
> }
>
> public ProcessResult executeBrowser(List<String> otherargs, String resource) throws Exception {
> - ProcessWrapper rpw = new ProcessWrapper(getBrowserLocation(), otherargs, getUrlUponThisInstance(resource), null, null, null);
> + ProcessWrapper rpw = new ProcessWrapper(getBrowserLocation(), otherargs, getUrlUponThisInstance(resource));
> rpw.setReactingProcess(getCurrentBrowser());//current browser may be null, but it does not metter
> return rpw.execute();
> }
> @@ -603,8 +607,16 @@
> return rpw.execute();
> }
>
> + public ProcessResult executeBrowser(List<String> otherargs, String resource, List<ContentReaderListener> stdoutl, List<ContentReaderListener> stderrl) throws Exception {
> + ProcessWrapper rpw = new ProcessWrapper(getBrowserLocation(), otherargs, getUrlUponThisInstance(resource), stdoutl, stderrl, null);
> + rpw.setReactingProcess(getCurrentBrowser());// current browser may be
> + // null, but it does not
> + // matter
> + return rpw.execute();
What an nasty indentation. Perhaps you have ide configured to use tabs as tabs? (should be tabs as
spaces, I think tab is 4sapces by default for ITW)
But maybe just email transformation messed it up. Then please ignore this...
> + }
> +
> public ProcessResult executeBrowser(Browser b, List<String> otherargs, String resource) throws Exception {
> - ProcessWrapper rpw = new ProcessWrapper(b.getBin(), otherargs, getUrlUponThisInstance(resource), null, null, null);
> + ProcessWrapper rpw = new ProcessWrapper(b.getBin(), otherargs, getUrlUponThisInstance(resource));
> rpw.setReactingProcess(b);
> return rpw.execute();
> }
> @@ -615,6 +627,12 @@
> return rpw.execute();
> }
>
> + public ProcessResult executeBrowser(Browser b, List<String> otherargs, String resource, List<ContentReaderListener> stdoutl, List<ContentReaderListener> stderrl) throws Exception {
> + ProcessWrapper rpw = new ProcessWrapper(b.getBin(), otherargs, getUrlUponThisInstance(resource), stdoutl, stderrl, null);
> + rpw.setReactingProcess(b);
> + return rpw.execute();
> + }
> +
> /**
> * Create resource on http, on 'localhost' on port on which this cached instance is running
> * @param resource
> @@ -661,7 +679,7 @@
> * @throws Exception
> */
> public static ProcessResult executeProcessUponURL(String toBeExecuted, List<String> otherargs, URL u) throws Exception {
> - return new ProcessWrapper(toBeExecuted, otherargs, u, null, null, null).execute();
> + return new ProcessWrapper(toBeExecuted, otherargs, u).execute();
> }
>
> public static ProcessResult executeProcessUponURL(String toBeExecuted, List<String> otherargs, URL u, ContentReaderListener stdoutl, ContentReaderListener stderrl) throws Exception {
>
Otherwise the changes have sense. Please double check that all the affected codes by small
refactoring you included in this patch have no impact to compilation/runtime of reproducers!
Thankx for dooing this
J.
More information about the distro-pkg-dev
mailing list