A little java/jar cleanup

* The Jar test is split into requires-jdk and not.
* Move some inline tool definitions to fixture files.
* Reformat some stuff.
* Some linting and commenting in Jar tool.
* Enabled test for a non-java file when Jar source is a directory
  (was commented out) - needed to add a Command to copy the file
  to the directory where the class files were generated.

Signed-off-by: Mats Wichmann <mats@linux.com>

7 files changed, 224 insertions, 265 deletions

diff --git a/SCons/Tool/jar.py b/SCons/Tool/jar.py
index a19c40a87..7f58dc3ad 100644
--- a/SCons/Tool/jar.py
+++ b/SCons/Tool/jar.py
@@ -29,15 +29,19 @@ selection method.
 """
 
 import os
+from typing import List
 
+import SCons.Node
+import SCons.Node.FS
 import SCons.Subst
+import SCons.Tool
 import SCons.Util
 import SCons.Warnings
 from SCons.Node.FS import _my_normcase
 from SCons.Tool.JavaCommon import get_java_install_dirs
 
 
-def jarSources(target, source, env, for_signature):
+def jarSources(target, source, env, for_signature) -> List[str]:
     """Only include sources that are not a manifest file."""
     try:
         env['JARCHDIR']
@@ -51,21 +55,22 @@ def jarSources(target, source, env, for_signature):
     result = []
     for src in source:
         contents = src.get_text_contents()
-        if not contents.startswith("Manifest-Version"):
-            if jarchdir_set:
-                _chdir = jarchdir
-            else:
-                try:
-                    _chdir = src.attributes.java_classdir
-                except AttributeError:
-                    _chdir = None
-            if _chdir:
-                # If we are changing the dir with -C, then sources should
-                # be relative to that directory.
-                src = SCons.Subst.Literal(src.get_path(_chdir))
-                result.append('-C')
-                result.append(_chdir)
-            result.append(src)
+        if contents.startswith("Manifest-Version"):
+            continue
+        if jarchdir_set:
+            _chdir = jarchdir
+        else:
+            try:
+                _chdir = src.attributes.java_classdir
+            except AttributeError:
+                _chdir = None
+        if _chdir:
+            # If we are changing the dir with -C, then sources should
+            # be relative to that directory.
+            src = SCons.Subst.Literal(src.get_path(_chdir))
+            result.append('-C')
+            result.append(_chdir)
+        result.append(src)
     return result
 
 def jarManifest(target, source, env, for_signature):
@@ -76,7 +81,7 @@ def jarManifest(target, source, env, for_signature):
             return src
     return ''
 
-def jarFlags(target, source, env, for_signature):
+def jarFlags(target, source, env, for_signature) -> str:
     """If we have a manifest, make sure that the 'm'
     flag is specified."""
     jarflags = env.subst('$JARFLAGS', target=target, source=source)
@@ -88,16 +93,19 @@ def jarFlags(target, source, env, for_signature):
             break
     return jarflags
 
-def Jar(env, target = None, source = [], *args, **kw):
-    """
-    A pseudo-Builder wrapper around the separate Jar sources{File,Dir}
-    Builders.
+def Jar(env, target=None, source=[], *args, **kw):
+    """The Jar Builder.
+
+    This is a pseudo-Builder wrapper around the separate jar builders
+    depending on whether the sources are a file list or a directory.
     """
+    # TODO: W1113: Keyword argument before variable positional arguments list in the definition of Jar function
+    # TODO: W0102: Dangerous default value [] as argument
 
     # jar target should not be a list so assume they passed
     # no target and want implicit target to be made and the arg
     # was actaully the list of sources
-    if SCons.Util.is_List(target) and source == []:
+    if SCons.Util.is_List(target) and source is None:
         SCons.Warnings.warn(
             SCons.Warnings.SConsWarning,
             "Making implicit target jar file, and treating the list as sources"
@@ -105,14 +113,14 @@ def Jar(env, target = None, source = [], *args, **kw):
         source = target
         target = None
 
-    # mutiple targets pass so build each target the same from the
+    # mutiple targets passed so build each target the same from the
     # same source
     #TODO Maybe this should only be done once, and the result copied
     #     for each target since it should result in the same?
     if SCons.Util.is_List(target) and SCons.Util.is_List(source):
         jars = []
         for single_target in target:
-            jars += env.Jar( target = single_target, source = source, *args, **kw)
+            jars += env.Jar(target=single_target, source=source, *args, **kw)
         return jars
 
     # they passed no target so make a target implicitly
@@ -121,6 +129,7 @@ def Jar(env, target = None, source = [], *args, **kw):
             # make target from the first source file
             target = os.path.splitext(str(source[0]))[0] + env.subst('$JARSUFFIX')
         except:
+            # TODO: W0702: No exception type(s) specified
             # something strange is happening but attempt anyways
             SCons.Warnings.warn(
                 SCons.Warnings.SConsWarning,
@@ -143,68 +152,68 @@ def Jar(env, target = None, source = [], *args, **kw):
     # if its already a class file then it can be used as a
     # source for jar, otherwise turn it into a class file then
     # return the source
-    def file_to_class(s):
-        if _my_normcase(str(s)).endswith(java_suffix):
-            return env.JavaClassFile(source = s, *args, **kw)
-        else:
-            return [env.fs.File(s)]
+    def file_to_class(src):
+        if _my_normcase(str(src)).endswith(java_suffix):
+            return env.JavaClassFile(source=src, *args, **kw)
+        return [env.fs.File(src)]
 
     # function for calling the JavaClassDir builder if a directory is
     # passed as a source to Jar builder. The JavaClassDir builder will
-    # return an empty list if there were not target classes built from
+    # return an empty list if there were no target classes built from
     # the directory, in this case assume the user wanted the directory
     # copied into the jar as is (it contains other files such as
-    # resources or class files compiled from proir commands)
+    # resources or class files compiled from prior commands)
     # TODO: investigate the expexcted behavior for directories that
     #       have mixed content, such as Java files along side other files
     #       files.
-    def dir_to_class(s):
-        dir_targets = env.JavaClassDir(source = s, *args, **kw)
+    def dir_to_class(src):
+        dir_targets = env.JavaClassDir(source=src, *args, **kw)
         if dir_targets == []:
             # no classes files could be built from the source dir
             # so pass the dir as is.
-            return [env.fs.Dir(s)]
-        else:
-            return dir_targets
+            return [env.fs.Dir(src)]
+        return dir_targets
 
     # loop through the sources and handle each accordingly
     # the goal here is to get all the source files into a class
     # file or a directory that contains class files
-    for s in SCons.Util.flatten(source):
-        s = env.subst(s)
-        if isinstance(s, SCons.Node.FS.Base):
-            if isinstance(s, SCons.Node.FS.File):
+    for src in SCons.Util.flatten(source):
+        src = env.subst(src)
+        if isinstance(src, SCons.Node.FS.Base):
+            if isinstance(src, SCons.Node.FS.File):
                 # found a file so make sure its a class file
-                target_nodes.extend(file_to_class(s))
+                target_nodes.extend(file_to_class(src))
             else:
                 # found a dir so get the class files out of it
-                target_nodes.extend(dir_to_class(s))
+                target_nodes.extend(dir_to_class(src))
         else:
             try:
                 # source is string try to convert it to file
-                target_nodes.extend(file_to_class(env.fs.File(s)))
+                target_nodes.extend(file_to_class(env.fs.File(src)))
                 continue
             except:
+                # TODO: W0702: No exception type(s) specified
                 pass
 
             try:
                 # source is string try to covnert it to dir
-                target_nodes.extend(dir_to_class(env.fs.Dir(s)))
+                target_nodes.extend(dir_to_class(env.fs.Dir(src)))
                 continue
             except:
+                # TODO: W0702: No exception type(s) specified
                 pass
 
             SCons.Warnings.warn(
                 SCons.Warnings.SConsWarning,
-                ("File: " + str(s)
+                ("File: " + str(src)
                 + " could not be identified as File or Directory, skipping.")
             )
 
-    # at this point all our sources have been converted to classes or directories of class
-    # so pass it to the Jar builder
-    return env.JarFile(target = target, source = target_nodes, *args, **kw)
+    # at this point all our sources have been converted to classes or
+    # directories of class so pass it to the Jar builder
+    return env.JarFile(target=target, source=target_nodes, *args, **kw)
 
-def generate(env):
+def generate(env) -> None:
     """Add Builders and construction variables for jar to an Environment."""
     SCons.Tool.CreateJarBuilder(env)
 
@@ -222,14 +231,14 @@ def generate(env):
             jar_bin_dir = os.path.dirname(jar)
             env.AppendENVPath('PATH', jar_bin_dir)
 
-    env['JAR']        = 'jar'
-    env['JARFLAGS']   = SCons.Util.CLVar('cf')
-    env['_JARFLAGS']  = jarFlags
+    env['JAR'] = 'jar'
+    env['JARFLAGS'] = SCons.Util.CLVar('cf')
+    env['_JARFLAGS'] = jarFlags
     env['_JARMANIFEST'] = jarManifest
     env['_JARSOURCES'] = jarSources
-    env['_JARCOM']    = '$JAR $_JARFLAGS $TARGET $_JARMANIFEST $_JARSOURCES'
-    env['JARCOM']     = "${TEMPFILE('$_JARCOM','$JARCOMSTR')}"
-    env['JARSUFFIX']  = '.jar'
+    env['_JARCOM'] = '$JAR $_JARFLAGS $TARGET $_JARMANIFEST $_JARSOURCES'
+    env['JARCOM'] = "${TEMPFILE('$_JARCOM','$JARCOMSTR')}"
+    env['JARSUFFIX'] = '.jar'
 
 def exists(env):
     # As reported by Jan Nijtmans in issue #2730, the simple
@@ -238,7 +247,7 @@ def exists(env):
     # stop trying to detect an executable (analogous to the
     # javac Builder).
     # TODO: Come up with a proper detect() routine...and enable it.
-    return 1
+    return True
 
 # Local Variables:
 # tab-width:4
diff --git a/test/Java/JAR.py b/test/Java/JAR.py
index bc52a8953..2d67717fe 100644
--- a/test/Java/JAR.py
+++ b/test/Java/JAR.py
@@ -23,6 +23,12 @@
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+"""
+Test Jar builder.
+
+These tests require a findable/working Java subsystem.
+"""
+
 import os
 import TestSCons
 
@@ -30,115 +36,23 @@ _python_ = TestSCons._python_
 
 test = TestSCons.TestSCons()
 
-# Keep this logic because it skips the test if javac or jar not found.
 where_javac, java_version = test.java_where_javac()
 where_jar = test.java_where_jar()
 
-test.write('myjar.py', r"""
-import sys
-args = sys.argv[1:]
-while args:
-    a = args[0]
-    if a == 'cf':
-        out = args[1]
-        args = args[1:]
-    else:
-        break
-    args = args[1:]
-outfile = open(out, 'w')
-for file in args:
-    infile = open(file, 'r')
-    for l in infile.readlines():
-        if l[:7] != '/*jar*/':
-            outfile.write(l)
-sys.exit(0)
-""")
-
-test.write('SConstruct', """
-DefaultEnvironment(tools=[])
-env = Environment(tools = ['jar'],
-                  JAR = r'%(_python_)s myjar.py')
-env.Jar(target = 'test1.jar', source = 'test1.class')
-""" % locals())
-
-test.write('test1.class', """\
-test1.class
-/*jar*/
-line 3
-""")
-
-test.run(arguments='.', stderr=None)
-
-test.must_match('test1.jar', "test1.class\nline 3\n", mode='r')
-
-if os.path.normcase('.class') == os.path.normcase('.CLASS'):
-
-    test.write('SConstruct', """
-DefaultEnvironment(tools=[])
-env = Environment(tools = ['jar'],
-                  JAR = r'%(_python_)s myjar.py')
-env.Jar(target = 'test2.jar', source = 'test2.CLASS')
-""" % locals())
-
-    test.write('test2.CLASS', """\
-test2.CLASS
-/*jar*/
-line 3
-""")
-
-    test.run(arguments='.', stderr=None)
-
-    test.must_match('test2.jar', "test2.CLASS\nline 3\n", mode='r')
-
-test.write('myjar2.py', r"""
-import sys
-f=open(sys.argv[2], 'w')
-f.write(" ".join(sys.argv[1:]))
-f.write("\n")
-f.close()
-sys.exit(0)
-""")
-
-test.write('SConstruct', """
-DefaultEnvironment(tools=[])
-env = Environment(tools = ['jar'],
-                  JAR = r'%(_python_)s myjar2.py',
-                  JARFLAGS='cvf')
-env.Jar(target = 'classes.jar', source = [ 'testdir/bar.class',
-                                           'foo.mf' ],
-        TESTDIR='testdir',
-        JARCHDIR='$TESTDIR')
-""" % locals())
-
-test.subdir('testdir')
-test.write(['testdir', 'bar.class'], 'foo')
-test.write('foo.mf',
-           """Manifest-Version : 1.0
-           blah
-           blah
-           blah
-           """)
-test.run(arguments='classes.jar')
-test.must_match('classes.jar',
-                'cvfm classes.jar foo.mf -C testdir bar.class\n', mode='r')
-
 test.file_fixture('wrapper_with_args.py')
 
-test.write('SConstruct', """
+test.write('SConstruct', """\
 DefaultEnvironment(tools=[])
-foo = Environment(tools = ['javac', 'jar'])
+foo = Environment(tools=['javac', 'jar'])
 # jar = foo.Dictionary('JAR')
-bar = foo.Clone(JAR = r'%(_python_)s wrapper_with_args.py jar')
-foo.Java(target = 'classes', source = 'com/sub/foo')
-bar.Java(target = 'classes', source = 'com/sub/bar')
-foo.Jar(target = 'foo', source = 'classes/com/sub/foo')
-bar.Jar(target = 'bar', source = Dir('classes/com/sub/bar'))
+bar = foo.Clone(JAR=r'%(_python_)s wrapper_with_args.py jar')
+foo.Java(target='classes', source='com/sub/foo')
+bar.Java(target='classes', source='com/sub/bar')
+foo.Jar(target='foo', source='classes/com/sub/foo')
+bar.Jar(target='bar', source=Dir('classes/com/sub/bar'))
 """ % locals())
 
-test.subdir('com',
-            ['com', 'sub'],
-            ['com', 'sub', 'foo'],
-            ['com', 'sub', 'bar'])
+test.subdir('com', ['com', 'sub'], ['com', 'sub', 'foo'], ['com', 'sub', 'bar'])
 
 test.write(['com', 'sub', 'foo', 'Example1.java'], """\
 package com.sub.foo;
@@ -228,13 +142,12 @@ test.run(arguments = '.')
 
 expected_wrapper_out = "wrapper_with_args.py jar cf bar.jar classes/com/sub/bar\n"
 expected_wrapper_out = expected_wrapper_out.replace('/', os.sep)
-test.must_match('wrapper.out',
-                expected_wrapper_out % locals(), mode='r')
+test.must_match('wrapper.out', expected_wrapper_out % locals(), mode='r')
 
 test.must_exist('foo.jar')
 test.must_exist('bar.jar')
 
-test.up_to_date(arguments = '.')
+test.up_to_date(arguments='.')
 
 #######
 # test java source files as source to Jar builder
@@ -250,22 +163,43 @@ test.write(['testdir2', 'SConstruct'], """
 DefaultEnvironment(tools=[])
 
 foo = Environment()
-foo.Jar(target = 'foobar', source = [
-    'com/javasource/JavaFile1.java', 
-    'com/javasource/JavaFile2.java',
-    'com/javasource/JavaFile3.java'
-])
-foo.Jar(target = ['foo', 'bar'], source = [
-    'com/javasource/JavaFile1.java', 
-    'com/javasource/JavaFile2.java',
-    'com/javasource/JavaFile3.java'
-])
-foo.Command("foobarTest", [], Mkdir("foobarTest") )
-foo.Command('foobarTest/com/javasource/JavaFile3.java', 'foobar.jar', foo['JAR'] + ' xvf ../foobar.jar', chdir='foobarTest')
-foo.Command("fooTest", [], Mkdir("fooTest") )
-foo.Command('fooTest/com/javasource/JavaFile3.java', 'foo.jar', foo['JAR'] + ' xvf ../foo.jar', chdir='fooTest')
-foo.Command("barTest", [], Mkdir("barTest") )
-foo.Command('barTest/com/javasource/JavaFile3.java', 'bar.jar', foo['JAR'] + ' xvf ../bar.jar', chdir='barTest')
+foo.Jar(
+    target='foobar',
+    source=[
+        'com/javasource/JavaFile1.java',
+        'com/javasource/JavaFile2.java',
+        'com/javasource/JavaFile3.java',
+    ],
+)
+foo.Jar(
+    target=['foo', 'bar'],
+    source=[
+        'com/javasource/JavaFile1.java',
+        'com/javasource/JavaFile2.java',
+        'com/javasource/JavaFile3.java',
+    ],
+)
+foo.Command("foobarTest", [], Mkdir("foobarTest"))
+foo.Command(
+    'foobarTest/com/javasource/JavaFile3.java',
+    'foobar.jar',
+    foo['JAR'] + ' xvf ../foobar.jar',
+    chdir='foobarTest',
+)
+foo.Command("fooTest", [], Mkdir("fooTest"))
+foo.Command(
+    'fooTest/com/javasource/JavaFile3.java',
+    'foo.jar',
+    foo['JAR'] + ' xvf ../foo.jar',
+    chdir='fooTest',
+)
+foo.Command("barTest", [], Mkdir("barTest"))
+foo.Command(
+    'barTest/com/javasource/JavaFile3.java',
+    'bar.jar',
+    foo['JAR'] + ' xvf ../bar.jar',
+    chdir='barTest',
+)
 """)
 
 test.write(['testdir2', 'com', 'javasource', 'JavaFile1.java'], """\
@@ -309,11 +243,7 @@ public class JavaFile3
 # use regex . for dirsep so this will work on both windows and other platforms.
 expect = ".*jar cf foo.jar -C com.javasource.JavaFile1 com.javasource.JavaFile1.class -C com.javasource.JavaFile2 com.javasource.JavaFile2.class -C com.javasource.JavaFile3 com.javasource.JavaFile3.class.*"
 
-test.run(chdir='testdir2',	
-         match=TestSCons.match_re_dotall,	
-         stdout = expect)
-
-
+test.run(chdir='testdir2', match=TestSCons.match_re_dotall, stdout=expect)
 
 #test single target jar
 test.must_exist(['testdir2','foobar.jar'])
@@ -338,12 +268,14 @@ test.must_exist(['testdir2', 'barTest', 'com', 'javasource', 'JavaFile3.class'])
 # test list of lists
 
 # make some directories to test in
-test.subdir('listOfLists',
-            ['manifest_dir'],
-            ['listOfLists', 'src'],
-            ['listOfLists', 'src', 'com'],
-            ['listOfLists', 'src', 'com', 'javasource'],
-            ['listOfLists', 'src', 'com', 'resource'])
+test.subdir(
+    'listOfLists',
+    ['manifest_dir'],
+    ['listOfLists', 'src'],
+    ['listOfLists', 'src', 'com'],
+    ['listOfLists', 'src', 'com', 'javasource'],
+    ['listOfLists', 'src', 'com', 'resource'],
+)
 
 # test varient dir and lists of lists
 test.write(['listOfLists', 'SConstruct'], """
@@ -352,15 +284,26 @@ DefaultEnvironment(tools=[])
 foo = Environment()
 foo.VariantDir('build', 'src', duplicate=0)
 foo.VariantDir('test', '../manifest_dir', duplicate=0)
-sourceFiles = ["src/com/javasource/JavaFile1.java", "src/com/javasource/JavaFile2.java", "src/com/javasource/JavaFile3.java",]
+sourceFiles = [
+    "src/com/javasource/JavaFile1.java",
+    "src/com/javasource/JavaFile2.java",
+    "src/com/javasource/JavaFile3.java",
+]
 list_of_class_files = foo.Java('build', source=sourceFiles)
 resources = ['build/com/resource/resource1.txt', 'build/com/resource/resource2.txt']
 for resource in resources:
-    foo.Command(resource, list_of_class_files, Copy(resource, resource.replace('build','src')))
+    foo.Command(
+        resource, list_of_class_files, Copy(resource, resource.replace('build', 'src'))
+    )
 contents = [list_of_class_files, resources]
-foo.Jar(target = 'lists', source = contents + ['test/MANIFEST.mf'], JARCHDIR='build')
-foo.Command("listsTest", [], Mkdir("listsTest") )
-foo.Command('listsTest/src/com/javasource/JavaFile3.java', 'lists.jar', foo['JAR'] + ' xvf ../lists.jar', chdir='listsTest')
+foo.Jar(target='lists', source=contents + ['test/MANIFEST.mf'], JARCHDIR='build')
+foo.Command("listsTest", [], Mkdir("listsTest"))
+foo.Command(
+    'listsTest/src/com/javasource/JavaFile3.java',
+    'lists.jar',
+    foo['JAR'] + ' xvf ../lists.jar',
+    chdir='listsTest',
+)
 """)
 
 test.write(['listOfLists', 'src', 'com', 'javasource', 'JavaFile1.java'], """\
@@ -431,25 +374,34 @@ test.must_contain(['listOfLists', 'listsTest', 'META-INF', 'MANIFEST.MF'], b"MyM
 # test different style of passing in dirs
 
 # make some directories to test in
-test.subdir('testdir3',
-            ['testdir3', 'com'],
-            ['testdir3', 'com', 'sub'],
-            ['testdir3', 'com', 'sub', 'foo'],
-            ['testdir3', 'com', 'sub', 'bar'])
+test.subdir(
+    'testdir3',
+    ['testdir3', 'com'],
+    ['testdir3', 'com', 'sub'],
+    ['testdir3', 'com', 'sub', 'foo'],
+    ['testdir3', 'com', 'sub', 'bar'],
+)
 
 # Create the jars then extract them back to check contents
 test.write(['testdir3', 'SConstruct'], """
 DefaultEnvironment(tools=[])
 
 foo = Environment()
-bar = foo.Clone()
-foo.Java(target = 'classes', source = 'com/sub/foo')
-bar.Java(target = 'classes', source = 'com/sub/bar')
-foo.Jar(target = 'foo', source = 'classes/com/sub/foo', JARCHDIR='classes')
-bar.Jar(target = 'bar', source = Dir('classes/com/sub/bar'), JARCHDIR='classes')
-foo.Command("fooTest", 'foo.jar', Mkdir("fooTest") )
+foo_cls = foo.Java(target='classes', source='com/sub/foo')
+foo_res = 'classes/com/sub/foo/NonJava.txt'
+foo_res_src = 'com/sub/foo/NonJava.txt'
+foo.Command(foo_res, foo_cls, Copy(foo_res, foo_res_src))
+foo.Jar(target='foo', source='classes/com/sub/foo', JARCHDIR='classes')
+foo.Command("fooTest", 'foo.jar', Mkdir("fooTest"))
 foo.Command('doesnt_exist1', "fooTest", foo['JAR'] + ' xvf ../foo.jar', chdir='fooTest')
-bar.Command("barTest", 'bar.jar', Mkdir("barTest") )
+
+bar = foo.Clone()
+bar_cls = bar.Java(target='classes', source='com/sub/bar')
+bar_res = 'classes/com/sub/bar/NonJava.txt'
+bar_res_src = 'com/sub/bar/NonJava.txt'
+bar.Command(bar_res, bar_cls, Copy(bar_res, bar_res_src))
+bar.Jar(target='bar', source=Dir('classes/com/sub/bar'), JARCHDIR='classes')
+bar.Command("barTest", 'bar.jar', Mkdir("barTest"))
 bar.Command('doesnt_exist2', 'barTest', bar['JAR'] + ' xvf ../bar.jar', chdir='barTest')
 """)
 
@@ -549,7 +501,6 @@ test.run(chdir='testdir3')
 
 # check the output and make sure the java files got converted to classes
 
-
 # make sure there are class in the jar
 test.must_exist(['testdir3','foo.jar'])
 test.must_exist(['testdir3', 'fooTest', 'com', 'sub', 'foo', 'Example1.class'])
@@ -557,7 +508,7 @@ test.must_exist(['testdir3', 'fooTest', 'com', 'sub', 'foo', 'Example2.class'])
 test.must_exist(['testdir3', 'fooTest', 'com', 'sub', 'foo', 'Example3.class'])
 # TODO: determine expected behavior with resource files, should they be 
 #       automatically copied in or specified in seperate commands
-#test.must_exist(['testdir3', 'fooTest', 'com', 'sub', 'foo', 'NonJava.txt'])
+test.must_exist(['testdir3', 'fooTest', 'com', 'sub', 'foo', 'NonJava.txt'])
 
 # make sure both jars got createds
 test.must_exist(['testdir3','bar.jar'])
@@ -566,11 +517,10 @@ test.must_exist(['testdir3', 'barTest', 'com', 'sub', 'bar', 'Example5.class'])
 test.must_exist(['testdir3', 'barTest', 'com', 'sub', 'bar', 'Example6.class'])
 # TODO: determine expected behavior with resource files, should they be 
 #       automatically copied in or specified in seperate commands
-#test.must_exist(['testdir3', 'fooTest', 'com', 'sub', 'bar', 'NonJava.txt'])
+test.must_exist(['testdir3', 'barTest', 'com', 'sub', 'bar', 'NonJava.txt'])
 
 test.pass_test()
 
-
 # Local Variables:
 # tab-width:4
 # indent-tabs-mode:nil
diff --git a/test/Java/JAVAC.py b/test/Java/JAVAC.py
index 20f7f9676..f303e8775 100644
--- a/test/Java/JAVAC.py
+++ b/test/Java/JAVAC.py
@@ -25,6 +25,8 @@
 
 """
 Test setting the JAVAC variable.
+
+This test does not require a JDK to operate.
 """
 
 import os
@@ -35,33 +37,11 @@ _python_ = TestSCons._python_
 
 test = TestSCons.TestSCons()
 
+test.file_fixture(['Java-fixture', 'myjavac.py'])
 
-
-test.write('myjavac.py', r"""
-import sys
-args = sys.argv[1:]
-while args:
-    a = args[0]
-    if a == '-d':
-        args = args[1:]
-    elif a == '-sourcepath':
-        args = args[1:]
-    else:
-        break
-    args = args[1:]
-for file in args:
-    infile = open(file, 'r')
-    outfile = open(file[:-5] + '.class', 'w')
-    for l in infile.readlines():
-        if l[:9] != '/*javac*/':
-            outfile.write(l)
-sys.exit(0)
-""")
-
-test.write('SConstruct', """
-env = Environment(tools = ['javac'],
-                  JAVAC = r'%(_python_)s myjavac.py')
-env.Java(target = '.', source = '.')
+test.write('SConstruct', """\
+env = Environment(tools=['javac'], JAVAC=r'%(_python_)s myjavac.py')
+env.Java(target='.', source='.')
 """ % locals())
 
 test.write('test1.java', """\
@@ -71,15 +51,12 @@ line 3
 """)
 
 test.run(arguments='.', stderr=None)
-
 test.must_match('test1.class', "test1.java\nline 3\n", mode='r')
 
 if os.path.normcase('.java') == os.path.normcase('.JAVA'):
-
     test.write('SConstruct', """\
-env = Environment(tools = ['javac'],
-                  JAVAC = r'%(_python_)s myjavac.py')
-env.Java(target = '.', source = '.')
+env = Environment(tools=['javac'], JAVAC=r'%(_python_)s myjavac.py')
+env.Java(target='.', source='.')
 """ % locals())
 
     test.write('test2.JAVA', """\
@@ -89,11 +66,8 @@ line 3
 """)
 
     test.run(arguments='.', stderr=None)
-
     test.must_match('test2.class', "test2.JAVA\nline 3\n", mode='r')
 
-
-
 test.pass_test()
 
 # Local Variables:
diff --git a/test/Java/Java-fixture/.exclude_tests b/test/Java/Java-fixture/.exclude_tests
new file mode 100644
index 000000000..37004e61f
--- /dev/null
+++ b/test/Java/Java-fixture/.exclude_tests
@@ -0,0 +1,2 @@
+myjar.py
+myjavac.py
diff --git a/test/Java/Java-fixture/myjar.py b/test/Java/Java-fixture/myjar.py
new file mode 100644
index 000000000..a47e3b07e
--- /dev/null
+++ b/test/Java/Java-fixture/myjar.py
@@ -0,0 +1,19 @@
+import fileinput
+import sys
+
+args = sys.argv[1:]
+while args:
+    arg = args[0]
+    if arg == 'cf':
+        out = args[1]
+        args = args[1:]
+    else:
+        break
+    args = args[1:]
+
+with open(out, 'wb') as ofp, fileinput.input(files=args, mode='rb') as ifp:
+    for line in ifp:
+        if not line.startswith(b'/*jar*/'):
+            ofp.write(line)
+
+sys.exit(0)
diff --git a/test/Java/Java-fixture/myjavac.py b/test/Java/Java-fixture/myjavac.py
new file mode 100644
index 000000000..0177c51a2
--- /dev/null
+++ b/test/Java/Java-fixture/myjavac.py
@@ -0,0 +1,22 @@
+import fileinput
+import sys
+
+args = sys.argv[1:]
+while args:
+    arg = args[0]
+    if arg == '-d':
+        args = args[1:]
+    elif arg == '-sourcepath':
+        args = args[1:]
+    else:
+        break
+    args = args[1:]
+
+for file in args:
+    out = file.lower().replace('.java', '.class')
+    with open(file, 'rb') as infile, open(out, 'wb') as outfile:
+        for line in infile:
+            if not line.startswith(b'/*javac*/'):
+                outfile.write(line)
+
+sys.exit(0)
diff --git a/test/Java/jar_not_in_PATH.py b/test/Java/jar_not_in_PATH.py
index 26e92ce25..05bb9c8fe 100644
--- a/test/Java/jar_not_in_PATH.py
+++ b/test/Java/jar_not_in_PATH.py
@@ -35,33 +35,16 @@ _python_ = TestSCons._python_
 
 test = TestSCons.TestSCons()
 
-test.write('myjar.py', r"""
-import sys
-args = sys.argv[1:]
-while args:
-    a = args[0]
-    if a == 'cf':
-        out = args[1]
-        args = args[1:]
-    else:
-        break
-    args = args[1:]
-outfile = open(out, 'wb')
-for file in args:
-    infile = open(file, 'rb')
-    for l in infile.readlines():
-        if l[:7] != '/*jar*/':
-            outfile.write(l)
-sys.exit(0)
-""")
+test.file_fixture(['Java-fixture', 'myjar.py'])
 
-test.write('SConstruct', """
+test.write('SConstruct', """\
 import os
-oldpath = os.environ.get('PATH','')
-env = Environment(ENV = {'PATH' : ['.']})
+
+oldpath = os.environ.get('PATH', '')
+env = Environment(ENV={'PATH': ['.']})
 env['ENV']['PATH'] = oldpath
 env['JAR'] = r'%(_python_)s ./myjar.py'
-env.Jar(target = 'test1.jar', source = 'test1.class')
+env.Jar(target='test1.jar', source='test1.class')
 """ % locals())
 
 test.write('test1.class', """\
@@ -70,7 +53,7 @@ test1.class
 line 3
 """)
 
-test.run(arguments = '.', stderr = None)
+test.run(arguments='.', stderr=None)
 
 test.must_exist('test1.jar')