[icedtea-web][rfc] Reproducer for PR1112

Adam Domurad adomurad at redhat.com
Tue Aug 28 07:38:16 PDT 2012


On Tue, 2012-08-28 at 13:01 +0200, Jiri Vanek wrote:
> On 08/27/2012 09:22 PM, Adam Domurad wrote:
> > Updated patch attached.
> >
> 
> Thank you for update. It looks more then good. Nice example for future custom ones!
> 
> Three nitpicks which you do not need to follow if you do not want.
> 
> >
> >
> ...snip...
> > new file mode 100644
> > --- /dev/null
> > +++ b/tests/reproducers/custom/AdditionalJarsInMetaInfIndexList/srcs/Makefile
> > @@ -0,0 +1,63 @@
> > +TESTNAME=AdditionalJarsInMetaInfIndexList
> > +ARCHIVE_TEST_FOLDER=archive_tag_folder_test
> > +
> > +JAVAC_CLASSPATH=$(JNLP_TESTS_ENGINE_DIR):$(NETX_DIR)/lib/classes.jar
> > +KEYTOOL=$(BOOT_DIR)/bin/keytool
> > +JARSIGNER=$(BOOT_DIR)/bin/jarsigner
> > +JAVAC=$(BOOT_DIR)/bin/javac
> > +JAR=$(BOOT_DIR)/bin/jar
> > +
> > +# File used because the 'jar' command does not accept an empty file
> > +DUMMY_FILE=jar_dummy_content
> > +
> > +# Index jar causes main class jar to load
> > +INDEX_JAR_UNSIGNED=AdditionalJarsInMetaInfIndexListUnsigned.jar
> > +INDEX_JAR_SIGNED=AdditionalJarsInMetaInfIndexListSigned.jar
> > +
> > +MAINCLASS=LoadedViaMetaInfIndexList
> > +
> > +MAINCLASS_JAR_UNSIGNED=LoadedViaMetaInfIndexListUnsigned.jar
> > +MAINCLASS_JAR_SIGNED=LoadedViaMetaInfIndexListSigned.jar
> > +
> > +KEYALIAS=$(TESTNAME)_alias
> > +KEYSTORE=$(PRIVATE_KEYSTORE_NAME)
> > +TMPDIR:=$(shell mktemp -d)
> > +
> > +prepare-reproducer:
> > +	echo PREPARING REPRODUCER $(TESTNAME) in $(TMPDIR)
> > +
> > +	$(JAVAC) -d $(TMPDIR) -classpath $(JAVAC_CLASSPATH) $(MAINCLASS).java
> > +	
> > +	# Create the jars which have INDEX.LIST
> > +	cd $(TMPDIR) ; \
> > +	echo "This file exists because jar command does not take 0 args">  $(DUMMY_FILE) ; \
> > +	$(JAR) cvf $(INDEX_JAR_UNSIGNED) $(DUMMY_FILE)	; \
> > +	$(JAR) cvf $(INDEX_JAR_SIGNED) $(DUMMY_FILE) ;
> > +	
> 
> I was once told that it is good habit to keep targets without empty lines, as such lines *can* cause the folowing rest of target ignored. Not sure how true this is :)
I believe this is OK for gmake as long as you keep a tab at the start of
the line. Is this considered bad style ? It makes it a heck of a lot
more readable.
> 
> > +	# Create the jar which has the main-class
> > +	# and update JAR_WITH_INDEX_LIST's index
> > +	cd $(TMPDIR) ; \
> > +	$(JAR) cvf $(MAINCLASS_JAR_UNSIGNED) $(MAINCLASS).class ; \
> > +	$(JAR) cvf $(MAINCLASS_JAR_SIGNED) $(MAINCLASS).class  ; \
> > +	$(JAR) i $(TMPDIR)/$(INDEX_JAR_UNSIGNED) $(MAINCLASS_JAR_UNSIGNED) ; \
> > +	$(JAR) i $(TMPDIR)/$(INDEX_JAR_SIGNED) $(MAINCLASS_JAR_SIGNED) ;
> > +	
> > +	# Sign some of the jars for the signed jar test
> > +	cd $(TMPDIR) ; \
> > +	for jar_to_sign in $(MAINCLASS_JAR_SIGNED) $(INDEX_JAR_SIGNED); do \
> > +	   $(BOOT_DIR)/bin/jarsigner -keystore $(TOP_BUILD_DIR)/$(PRIVATE_KEYSTORE_NAME) -storepass  $(PRIVATE_KEYSTORE_PASS)  \
> > +	   -keypass $(PRIVATE_KEYSTORE_PASS) "$$jar_to_sign" $(TEST_CERT_ALIAS)_signed   ; \
> > +	done
> > +	
> > +	# Move jars into deployment directory
> > +	cd $(TMPDIR); \
> > +	mv $(INDEX_JAR_UNSIGNED) $(JNLP_TESTS_SERVER_DEPLOYDIR) ; \
> > +	mv $(INDEX_JAR_SIGNED) $(JNLP_TESTS_SERVER_DEPLOYDIR) ; \
> > +	mv $(MAINCLASS_JAR_UNSIGNED) $(JNLP_TESTS_SERVER_DEPLOYDIR) ; \
> > +	mv $(MAINCLASS_JAR_SIGNED) $(JNLP_TESTS_SERVER_DEPLOYDIR) ;
> > +	
> > +	echo PREPARED REPRODUCER $(TESTNAME), removing $(TMPDIR)
> > +	rm -rf $(TMPDIR)
> > +
> > +clean-reproducer:
> 
> Do you mind echo "Nothing to be cleaned for $(TESTNAME)" ?
Done
> 
> > +	
> > diff --git a/tests/reproducers/custom/AdditionalJarsInMetaInfIndexList/testcases/AdditionalJarsInMetaInfIndexListTests.java b/tests/reproducers/custom/AdditionalJarsInMetaInfIndexList/testcases/AdditionalJarsInMetaInfIndexListTests.java
> > new file mode 100644
> > --- /dev/null
> > +++ b/tests/reproducers/custom/AdditionalJarsInMetaInfIndexList/testcases/AdditionalJarsInMetaInfIndexListTests.java
> > @@ -0,0 +1,70 @@
> > +/* AdditionalJarsInMetaInfIndexListTests.java
> > +Copyright (C) 2011 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 static org.junit.Assert.*;
> > +
> > +import java.util.Arrays;
> > +import java.util.Collections;
> > +import java.util.List;
> > +
> > +import net.sourceforge.jnlp.ServerAccess;
> > +import net.sourceforge.jnlp.ProcessResult;
> > +import net.sourceforge.jnlp.annotations.Bug;
> > +import net.sourceforge.jnlp.browsertesting.BrowserTest;
> > +
> > +import org.junit.Test;
> > +
> > +public class AdditionalJarsInMetaInfIndexListTests extends BrowserTest {
> > +
> > +    private static ServerAccess server = new ServerAccess();
> > +    private static final List<String>  TRUSTALL = Collections.unmodifiableList(Arrays.asList(new String[] { "-Xtrustall" }));
> > +
> > +    @Test
> > +    @Bug(id = "PR1112")
> > +    public void SignedMetaInfIndexListTest() throws Exception {
> > +        final String CORRECT_EXEC = "Program Executed Correctly.";
> 
> This should be extracted to private/public static final constant
Done
> 
> > +        ProcessResult pr = server.executeJavawsHeadless("/AdditionalJarsInMetaInfIndexListSigned.jnlp");
> > +        assertTrue("LoadedViaMetaInfIndexList's stdout should contain " + CORRECT_EXEC + " but did not.", pr.stdout.contains(CORRECT_EXEC));
> > +    }
> > +
> > +    @Test
> > +    public void UnsignedMetaInfIndexListTest() throws Exception {
> > +        final String CORRECT_EXEC = "Program Executed Correctly.";
> > +        ProcessResult pr = server.executeJavawsHeadless(TRUSTALL, "/AdditionalJarsInMetaInfIndexListUnsigned.jnlp");
> > +        assertTrue("LoadedViaMetaInfIndexList's stdout should contain " + CORRECT_EXEC + " but did not.", pr.stdout.contains(CORRECT_EXEC));
> > +    }
> > +}
> 
> Thank you for digging in it, feel free to push (changelog, hg add, do not add to much ;) )
> 
> J.
> 
Thanks, I have pushed with your changes.




More information about the distro-pkg-dev mailing list