[groovy] branch master updated: JUnit 4 and other minor edits

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[groovy] branch master updated: JUnit 4 and other minor edits

emilles
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 7c26ce6  JUnit 4 and other minor edits
7c26ce6 is described below

commit 7c26ce6c81b258ae5d4b654d6c7ebc74d3030c5f
Author: Eric Milles <[hidden email]>
AuthorDate: Wed Nov 6 11:05:46 2019 -0600

    JUnit 4 and other minor edits
---
 src/main/groovy/groovy/grape/GrapeIvy.groovy | 118 ++++-----
 src/test/groovy/grape/GrapeIvyTest.groovy    | 355 ++++++++++++++-------------
 2 files changed, 233 insertions(+), 240 deletions(-)

diff --git a/src/main/groovy/groovy/grape/GrapeIvy.groovy b/src/main/groovy/groovy/grape/GrapeIvy.groovy
index be969b9..c1c637e 100644
--- a/src/main/groovy/groovy/grape/GrapeIvy.groovy
+++ b/src/main/groovy/groovy/grape/GrapeIvy.groovy
@@ -23,8 +23,6 @@ import org.apache.groovy.plugin.GroovyRunner
 import org.apache.groovy.plugin.GroovyRunnerRegistry
 import org.apache.ivy.Ivy
 import org.apache.ivy.core.IvyContext
-import org.apache.ivy.core.cache.ResolutionCacheManager
-import org.apache.ivy.core.event.IvyListener
 import org.apache.ivy.core.event.download.PrepareDownloadEvent
 import org.apache.ivy.core.event.resolve.StartResolveEvent
 import org.apache.ivy.core.module.descriptor.Configuration
@@ -52,7 +50,6 @@ import org.codehaus.groovy.reflection.ReflectionUtils
 import org.codehaus.groovy.runtime.m12n.ExtensionModuleScanner
 import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl
 
-import javax.xml.parsers.DocumentBuilderFactory
 import java.util.jar.JarFile
 import java.util.regex.Pattern
 import java.util.zip.ZipEntry
@@ -86,7 +83,6 @@ class GrapeIvy implements GrapeEngine {
     // we keep the settings so that addResolver can add to the resolver chain
     IvySettings settings
 
-    @SuppressWarnings('Instanceof')
     GrapeIvy() {
         // if we are already initialized, quit
         if (enableGrapes) return
@@ -104,9 +100,8 @@ class GrapeIvy implements GrapeEngine {
             settings.load(grapeConfig) // exploit multi-methods for convenience
         } catch (java.text.ParseException ex) {
             def configLocation = grapeConfig instanceof File ? grapeConfig.canonicalPath : grapeConfig.toString()
-            System.err.println "Local Ivy config file '$configLocation' appears corrupt - ignoring it and using default config instead\nError was: " + ex.message
-            grapeConfig = GrapeIvy.getResource('defaultGrapeConfig.xml')
-            settings.load(grapeConfig)
+            System.err.println "Local Ivy config file '$configLocation' appears corrupt - ignoring it and using default config instead\nError was: ${ex.message}"
+            settings.load(GrapeIvy.getResource('defaultGrapeConfig.xml'))
         }
 
         // set up the cache dirs
@@ -418,11 +413,9 @@ class GrapeIvy implements GrapeEngine {
     }
 
     ResolveReport getDependencies(Map args, IvyGrabRecord... grabRecords) {
-        ResolutionCacheManager cacheManager = ivyInstance.resolutionCacheManager
-
+        def cacheManager = ivyInstance.resolutionCacheManager
         def millis = System.currentTimeMillis()
-        def md = new DefaultModuleDescriptor(ModuleRevisionId
-                .newInstance('caller', 'all-caller', 'working' + millis.toString()[-2..-1]), 'integration', null, true)
+        def md = new DefaultModuleDescriptor(ModuleRevisionId.newInstance('caller', 'all-caller', 'working' + millis.toString()[-2..-1]), 'integration', null, true)
         md.addConfiguration(new Configuration('default'))
         md.lastModified = millis
 
@@ -431,30 +424,34 @@ class GrapeIvy implements GrapeEngine {
         for (IvyGrabRecord grabRecord : grabRecords) {
             def conf = grabRecord.conf ?: ['*']
             DefaultDependencyDescriptor dd = (DefaultDependencyDescriptor) md.dependencies.find {
-                it.dependencyRevisionId.equals(grabRecord.mrid)
+                it.dependencyRevisionId == grabRecord.mrid
             }
-            if (dd) {
-                addDependencyArtifactDescriptor(dd, grabRecord, conf)
-            } else {
-                dd = new DefaultDependencyDescriptor(md, grabRecord.mrid, grabRecord.force,
-                        grabRecord.changing, grabRecord.transitive)
+            if (!dd) {
+                dd = new DefaultDependencyDescriptor(md, grabRecord.mrid, grabRecord.force, grabRecord.changing, grabRecord.transitive)
                 conf.each { dd.addDependencyConfiguration('default', it) }
-                addDependencyArtifactDescriptor(dd, grabRecord, conf)
                 md.addDependency(dd)
             }
+
+            // the optional configuration typically does not extend the default or master configuration, so prevent grabbing the main artifact
+            if (Collections.singleton('optional') == (conf as Set)) continue
+
+            // add artifact descriptor to dependency descriptor
+            def dad = new DefaultDependencyArtifactDescriptor(dd, grabRecord.mrid.name, grabRecord.type ?: 'jar', grabRecord.ext ?: 'jar', null, grabRecord.classifier ? [classifier: grabRecord.classifier] : null)
+            conf.each { dad.addConfiguration(it) }
+            dd.addDependencyArtifact('default', dad)
         }
 
         // resolve grab and dependencies
-        ResolveOptions resolveOptions = new ResolveOptions().tap {
-            confs = DEF_CONFIG as String[]
-            outputReport = false
-            validate = (boolean) (args.containsKey('validate') ? args.validate : false)
-        }
+        def resolveOptions = new ResolveOptions(
+            confs: DEF_CONFIG as String[],
+            outputReport: false,
+            validate: (boolean) (args.containsKey('validate') ? args.validate : false)
+        )
         ivyInstance.settings.defaultResolver = args.autoDownload ? 'downloadGrapes' : 'cachedGrapes'
         if (args.disableChecksums) {
             ivyInstance.settings.setVariable('ivy.checksums', '')
         }
-        boolean reportDownloads = System.getProperty('groovy.grape.report.downloads', 'false') == 'true'
+        boolean reportDownloads = Boolean.getBoolean('groovy.grape.report.downloads')
         if (reportDownloads) {
             addIvyListener()
         }
@@ -493,39 +490,27 @@ class GrapeIvy implements GrapeEngine {
     }
 
     private addIvyListener() {
-        ivyInstance.eventManager.addIvyListener([progress: { ivyEvent ->
+        ivyInstance.eventManager.addIvyListener { ivyEvent ->
             switch (ivyEvent) {
-                case StartResolveEvent:
-                    ivyEvent.moduleDescriptor.dependencies.each { it ->
-                        def name = it.toString()
-                        if (!resolvedDependencies.contains(name)) {
-                            resolvedDependencies << name
-                            System.err.println "Resolving $name"
-                        }
+            case StartResolveEvent:
+                ivyEvent.moduleDescriptor.dependencies.each {
+                    def name = it.toString()
+                    if (!resolvedDependencies.contains(name)) {
+                        resolvedDependencies << name
+                        System.err.println "Resolving $name"
                     }
-                    break
-                case PrepareDownloadEvent:
-                    ivyEvent.artifacts.each { it ->
-                        def name = it.toString()
-                        if (!downloadedArtifacts.contains(name)) {
-                            downloadedArtifacts << name
-                            System.err.println "Preparing to download artifact $name"
-                        }
+                }
+                break
+            case PrepareDownloadEvent:
+                ivyEvent.artifacts.each {
+                    def name = it.toString()
+                    if (!downloadedArtifacts.contains(name)) {
+                        downloadedArtifacts << name
+                        System.err.println "Preparing to download artifact $name"
                     }
-                    break
+                }
+                break
             }
-        }] as IvyListener)
-    }
-
-    @CompileStatic
-    private void addDependencyArtifactDescriptor(DefaultDependencyDescriptor dd, IvyGrabRecord grabRecord, List<String> conf) {
-        // TODO: find out "unknown" reason and change comment below - also, confirm conf[0] check vs conf.contains('optional')
-        if (conf[0] != 'optional' || grabRecord.classifier) {
-            // for some unknown reason optional dependencies should not have an artifactDescriptor
-            def dad = new DefaultDependencyArtifactDescriptor(dd,
-                    grabRecord.mrid.name, grabRecord.type ?: 'jar', grabRecord.ext ?: 'jar', null, grabRecord.classifier ? [classifier: grabRecord.classifier] : null)
-            conf.each { dad.addConfiguration(it) }
-            dd.addDependencyArtifact('default', dad)
         }
     }
 
@@ -540,8 +525,7 @@ class GrapeIvy implements GrapeEngine {
                         // TODO handle other types? e.g. 'dlls'
                         def jardir = new File(moduleDir, 'jars')
                         if (!jardir.exists()) return
-                        def dbf = DocumentBuilderFactory.newInstance()
-                        def db = dbf.newDocumentBuilder()
+                        def db = javax.xml.parsers.DocumentBuilderFactory.newInstance().newDocumentBuilder()
                         def root = db.parse(ivyFile).documentElement
                         def publis = root.getElementsByTagName('publications')
                         for (int i = 0; i < publis.length; i++) {
@@ -784,20 +768,16 @@ class IvyGrabRecord {
     }
 
     @Override
-    boolean equals(Object obj) {
-        if (null == obj || obj.class != IvyGrabRecord) {
-            return false
+    boolean equals(Object that) {
+        if (that instanceof IvyGrabRecord) {
+            return (this.mrid == that.mrid)
+                && (this.conf == that.conf)
+                && (this.changing == that.changing)
+                && (this.transitive == that.transitive)
+                && (this.force == that.force)
+                && (this.classifier == that.classifier)
+                && (this.ext == that.ext)
+                && (this.type == that.type)
         }
-
-        IvyGrabRecord o = (IvyGrabRecord) obj
-
-        ((changing == o.changing)
-                && (transitive == o.transitive)
-                && (force == o.force)
-                && (mrid == o.mrid)
-                && (conf == o.conf)
-                && (classifier == o.classifier)
-                && (ext == o.ext)
-                && (type == o.type))
     }
 }
diff --git a/src/test/groovy/grape/GrapeIvyTest.groovy b/src/test/groovy/grape/GrapeIvyTest.groovy
index 0fb05b5..375d0c7 100644
--- a/src/test/groovy/grape/GrapeIvyTest.groovy
+++ b/src/test/groovy/grape/GrapeIvyTest.groovy
@@ -19,270 +19,284 @@
 package groovy.grape
 
 import org.codehaus.groovy.control.CompilationFailedException
-import gls.CompilableTestSupport
+import org.junit.BeforeClass
+import org.junit.Test
 
-class GrapeIvyTest extends CompilableTestSupport {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
 
-    GrapeIvyTest() {
+final class GrapeIvyTest {
+
+    private static Set<String> jarNames(GroovyClassLoader loader) {
+        loader.URLs.collect { url -> url.path.split('/')[-1] } as Set
+    }
+
+    @BeforeClass
+    static void setUpClass() {
         // make sure files are installed locally
-        [[groupId:'log4j', artifactId:'log4j', version:'1.1.3'],
+        [
+            [groupId:'log4j', artifactId:'log4j', version:'1.1.3'],
             [groupId:'org.apache.poi', artifactId:'poi', version:'3.7'],
             [groupId:'com.jidesoft', artifactId:'jide-oss', version:'2.2.12'],
-            [groupId:'org.apache.ivy', artifactId:'ivy', version:'2.0.0', conf:['default', 'optional']],
-            [groupId:'net.sf.json-lib', artifactId:'json-lib', version:'2.2.3', classifier:'jdk15']
-        ].each {
-            Grape.resolve([autoDownload:true, classLoader:new GroovyClassLoader()], it)
+            [groupId:'commons-lang', artifactId:'commons-lang', version:'2.6'],
+            [groupId:'org.neo4j', artifactId:'neo4j-kernel', version:'2.0.0-RC1'],
+            [groupId:'commons-digester', artifactId:'commons-digester', version:'2.1'],
+            [groupId:'net.sf.json-lib', artifactId:'json-lib', version:'2.2.3', classifier:'jdk15'],
+            [groupId:'org.apache.ivy', artifactId:'ivy', version:'2.0.0', conf:['default', 'optional']]
+        ].each { spec ->
+            Grape.resolve([autoDownload:true, classLoader:new GroovyClassLoader()], spec)
         }
     }
 
+    @Test
     void testSingleArtifact() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
-        Grape.grab(groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)', classLoader:loader)
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
+        Grape.grab(groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)', classLoader:shell.classLoader)
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testModuleWithDependencies() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class")
+            shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class')
         }
-        Grape.grab(groupId:'org.apache.poi', artifactId:'poi', version:'3.7', classLoader:loader)
-        assert shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class").name == 'org.apache.poi.POIDocument'
+        Grape.grab(groupId:'org.apache.poi', artifactId:'poi', version:'3.7', classLoader:shell.classLoader)
+        assert shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class').name == 'org.apache.poi.POIDocument'
     }
 
+    @Test
     void testMultipleDependencies() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class")
+            shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class')
         }
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
 
-        Grape.grab(classLoader:loader,
+        Grape.grab(classLoader:shell.classLoader,
             [groupId:'org.apache.poi', artifactId:'poi', version:'3.7'],
             [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])
 
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
-        assert shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class").name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class').name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testListDependencies() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class")
+            shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class')
         }
 
-        Grape.grab(classLoader:loader,
+        Grape.grab(classLoader:shell.classLoader,
             [groupId:'org.apache.poi', artifactId:'poi', version:'3.7'],
             [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])
 
-        def loadedDependencies = Grape.listDependencies(loader)
+        def loadedDependencies = Grape.listDependencies(shell.classLoader)
         assert loadedDependencies == [
             [group:'org.apache.poi', module:'poi', version:'3.7'],
             [group:'com.jidesoft', module:'jide-oss', version:'[2.2.1,2.3)']
         ]
     }
 
+    @Test
     void testGrabRefless() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
         shell.evaluate("new groovy.grape.Grape().grab(groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)')")
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testGrabScriptClass() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
         shell.evaluate("new groovy.grape.Grape().grab(groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)', refObject:this)")
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testGrabScriptLoader() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
-        shell.setVariable("loader", loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
+        shell.setVariable('loader', shell.classLoader)
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
         shell.evaluate("new groovy.grape.Grape().grab(groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)', classLoader:loader)")
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testGrabReflessMultiple() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class")
+            shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class')
         }
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
-        shell.evaluate("""new groovy.grape.Grape().grab([:],
+        shell.evaluate('''new groovy.grape.Grape().grab([:],
             [groupId:'org.apache.poi', artifactId:'poi', version:'3.7'],
-            [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])""")
+            [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])''')
 
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
-        assert shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class").name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class').name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testGrabScriptClassMultiple() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class")
+            shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class')
         }
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
-        shell.evaluate("""new groovy.grape.Grape().grab(refObject: this,
+        shell.evaluate('''new groovy.grape.Grape().grab(refObject: this,
             [groupId:'org.apache.poi', artifactId:'poi', version:'3.7'],
-            [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])""")
+            [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])''')
 
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
         assert shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class").name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testGrabScriptLoaderMultiple() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
-        shell.setVariable("loader", loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
+        shell.setVariable('loader', shell.classLoader)
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class")
+            shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class')
         }
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class")
+            shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class')
         }
-        shell.evaluate("""new groovy.grape.Grape().grab(classLoader:loader,
+        shell.evaluate('''new groovy.grape.Grape().grab(classLoader:loader,
             [groupId:'org.apache.poi', artifactId:'poi', version:'3.7'],
-            [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])""")
+            [groupId:'com.jidesoft', artifactId:'jide-oss', version:'[2.2.1,2.3)'])''')
 
-        assert shell.evaluate("import com.jidesoft.swing.JideSplitButton; JideSplitButton.class").name == 'com.jidesoft.swing.JideSplitButton';
-        assert shell.evaluate("import org.apache.poi.POIDocument; POIDocument.class").name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate('import org.apache.poi.POIDocument; POIDocument.class').name == 'org.apache.poi.POIDocument'
+        assert shell.evaluate('import com.jidesoft.swing.JideSplitButton; JideSplitButton.class').name == 'com.jidesoft.swing.JideSplitButton';
     }
 
+    @Test
     void testSerialGrabs() {
         GroovyClassLoader loader = new GroovyClassLoader()
         Grape.grab(groupId:'log4j', artifactId:'log4j', version:'1.1.3', classLoader:loader)
         Grape.grab(groupId:'org.apache.poi', artifactId:'poi', version:'3.7', classLoader:loader)
         def jars = jarNames(loader)
         // because poi asks for log4j 1.2.13, but we already have 1.1.3 so it won't be loaded
-        assert jars.contains ("log4j-1.1.3.jar")
-        assert !jars.contains ("log4j-1.2.13.jar")
+        assert jars.contains('log4j-1.1.3.jar')
+        assert !jars.contains('log4j-1.2.13.jar')
 
         Grape.grab(groupId:'log4j', artifactId:'log4j', version:'1.2.13', classLoader:loader)
         jars = jarNames(loader)
         // because log4j 1.1.3 was loaded first, 1.2.13 should not get loaded
         // even though it was explicitly asked for
-        assert jars.contains ("log4j-1.1.3.jar")
-        assert !jars.contains ("log4j-1.2.13.jar")
+        assert jars.contains('log4j-1.1.3.jar')
+        assert !jars.contains('log4j-1.2.13.jar')
     }
 
+    @Test
     void testConf() {
-        GroovyClassLoader loader = new GroovyClassLoader()
+        Set noJars = [
+        ]
+        Set coreJars = [
+            'ivy-2.0.0.jar'
+        ]
+        Set testJars = [
+            'commons-lang-2.3.jar',
+            'junit-3.8.2.jar'
+        ]
+        Set optionalJars = [
+            'ant-1.6.2.jar',
+            'ant-nodeps-1.6.2.jar',
+            'ant-trax-1.6.2.jar',
+            'commons-codec-1.2.jar',
+            'commons-httpclient-3.0.jar',
+            'commons-logging-1.0.4.jar',
+            'commons-vfs-1.0.jar',
+            'jsch-0.1.31.jar',
+            'junit-3.8.1.jar',
+            'oro-2.0.8.jar'
+        ]
 
-        def coreJars = ["ivy-2.0.0.jar"] as Set
-        def optionalJars = [
-                "ant-1.6.2.jar",
-                "ant-trax-1.6.2.jar",
-                "ant-nodeps-1.6.2.jar",
-                "commons-httpclient-3.0.jar",
-                "junit-3.8.1.jar",
-                "commons-codec-1.2.jar",
-                "oro-2.0.8.jar",
-                "commons-vfs-1.0.jar",
-                "commons-logging-1.0.4.jar",
-                "jsch-0.1.31.jar",
-            ]  as Set
-        def testJars = [
-                "junit-3.8.2.jar",
-                "commons-lang-2.3.jar",
-            ]  as Set
-
-        Grape.grab(groupId:'org.apache.ivy', artifactId:'ivy', version:'2.0.0', classLoader:loader, preserveFiles:true)
+        def loader = new GroovyClassLoader()
+        Grape.grab(groupId:'org.apache.ivy', artifactId:'ivy', version:'2.0.0', classLoader:loader)
         def jars = jarNames(loader)
-        assert coreJars - jars == Collections.EMPTY_SET, "assert that all core jars are present"
-        assert testJars - jars == testJars, "assert that no test jars are present"
-        assert optionalJars - jars == optionalJars, "assert that no optional jars are present"
-        assert jars == coreJars, "assert that no extraneous jars are present"
+        assert jars == coreJars, 'assert that no extraneous jars are present'
+        assert coreJars - jars == noJars, 'assert that all core jars are present'
+        assert testJars - jars == testJars, 'assert that no test jars are present'
+        assert optionalJars - jars == optionalJars, 'assert that no optional jars are present'
 
         loader = new GroovyClassLoader()
         Grape.grab(groupId:'org.apache.ivy', artifactId:'ivy', version:'2.0.0', conf:'optional', classLoader:loader)
         jars = jarNames(loader)
-        assert coreJars - jars == coreJars, "assert that no core jars are present"
-        assert testJars - jars == testJars, "assert that no test jars are present"
-        assert optionalJars - jars == Collections.EMPTY_SET, "assert that all optional jars are present"
-        assert jars == optionalJars, "assert that no extraneous jars are present"
+        assert jars == optionalJars, 'assert that no extraneous jars are present'
+        assert coreJars - jars == coreJars, 'assert that no core jars are present'
+        assert testJars - jars == testJars, 'assert that no test jars are present'
+        assert optionalJars - jars == noJars, 'assert that all optional jars are present'
 
         loader = new GroovyClassLoader()
         Grape.grab(groupId:'org.apache.ivy', artifactId:'ivy', version:'2.0.0', conf:['default', 'optional'], classLoader:loader)
         jars = jarNames(loader)
-        assert coreJars - jars == Collections.EMPTY_SET, "assert that all core jars are present"
-        assert testJars - jars == testJars, "assert that no test jars are present"
-        assert optionalJars - jars == Collections.EMPTY_SET, "assert that all optional jars are present"
-        assert jars == coreJars + optionalJars, "assert that no extraneous jars are present"
-    }
-
-    private static Set<String> jarNames(GroovyClassLoader loader) {
-        loader.getURLs().collect { URL it -> it.getPath().split('/')[-1] } as Set
+        assert coreJars - jars == noJars, 'assert that all core jars are present'
+        assert testJars - jars == testJars, 'assert that no test jars are present'
+        assert optionalJars - jars == noJars, 'assert that all optional jars are present'
+        assert jars == coreJars + optionalJars, 'assert that no extraneous jars are present'
     }
 
+    @Test
     void testClassifier() {
-        GroovyClassLoader loader = new GroovyClassLoader()
-        GroovyShell shell = new GroovyShell(loader)
+        def shell = new GroovyShell(new GroovyClassLoader())
         shouldFail(CompilationFailedException) {
-            shell.evaluate("import net.sf.json.JSON; JSON")
+            shell.evaluate('import net.sf.json.JSON; JSON')
         }
-        Grape.grab(groupId:'net.sf.json-lib', artifactId:'json-lib', version:'2.2.3', classifier:'jdk15', classLoader:loader)
-        assert shell.evaluate("import net.sf.json.JSON; JSON").name == 'net.sf.json.JSON'
+        Grape.grab(groupId:'net.sf.json-lib', artifactId:'json-lib', version:'2.2.3', classifier:'jdk15', classLoader:shell.classLoader)
+        assert shell.evaluate('import net.sf.json.JSON; JSON').name == 'net.sf.json.JSON'
     }
 
+    @Test
     void testClassifierWithConf() {
-        def coreJars = [
-                "json-lib-2.2.3-jdk15.jar",
-                "commons-lang-2.4.jar",
-                "commons-collections-3.2.jar",
-                "ezmorph-1.0.6.jar",
-                "commons-logging-1.1.1.jar",
-                "commons-beanutils-1.7.0.jar"
-            ] as Set
-        def optionalJars = [
-                "xercesImpl-2.6.2.jar",
-                "xmlParserAPIs-2.6.2.jar",
-                "groovy-all-1.5.7.jar",
-                "oro-2.0.8.jar",
-                "jruby-1.1.jar",
-                "junit-3.8.2.jar",
-                "ant-launcher-1.7.0.jar",
-                "xalan-2.7.0.jar",
-                "json-lib-2.2.3-jdk15.jar",
-                "ant-1.7.0.jar",
-                "xom-1.1.jar",
-                "jaxen-1.1-beta-8.jar",
-                "jdom-1.0.jar",
-                "jline-0.9.94.jar",
-                "log4j-1.2.14.jar",
-                "dom4j-1.6.1.jar"
-            ] as Set
+        Set coreJars = [
+            'json-lib-2.2.3-jdk15.jar',
+            'commons-beanutils-1.7.0.jar',
+            'commons-collections-3.2.jar',
+            'commons-lang-2.4.jar',
+            'commons-logging-1.1.1.jar',
+            'ezmorph-1.0.6.jar'
+        ]
+        Set optionalJars = [
+            'ant-1.7.0.jar',
+            'ant-launcher-1.7.0.jar',
+            'dom4j-1.6.1.jar',
+            'groovy-all-1.5.7.jar',
+            'jaxen-1.1-beta-8.jar',
+            'jdom-1.0.jar',
+            'jline-0.9.94.jar',
+            'jruby-1.1.jar',
+            'junit-3.8.2.jar',
+            'log4j-1.2.14.jar',
+            'oro-2.0.8.jar',
+            'xalan-2.7.0.jar',
+            'xercesImpl-2.6.2.jar',
+            'xmlParserAPIs-2.6.2.jar',
+            'xom-1.1.jar'
+        ]
 
-        GroovyClassLoader loader = new GroovyClassLoader()
-        Grape.grab(groupId:'net.sf.json-lib', artifactId:'json-lib', version:'2.2.3', classifier:'jdk15', classLoader:loader, preserveFiles:true)
+        def loader = new GroovyClassLoader()
+        Grape.grab(groupId:'net.sf.json-lib', artifactId:'json-lib', version:'2.2.3', classifier:'jdk15', classLoader:loader)
         assert jarNames(loader) == coreJars
 
         loader = new GroovyClassLoader()
@@ -294,8 +308,8 @@ class GrapeIvyTest extends CompilableTestSupport {
         assert jarNames(loader) == coreJars + optionalJars
     }
 
+    @Test // BeanUtils is a transitive dependency for Digester
     void testTransitiveShorthandControl() {
-        // BeanUtils is a transitive dependency for Digester
         assertScript '''
             @Grab('commons-digester:commons-digester:2.1')
             import org.apache.commons.digester.Digester
@@ -305,29 +319,26 @@ class GrapeIvyTest extends CompilableTestSupport {
         '''
     }
 
+    @Test
     void testTransitiveShorthandExpectFailure() {
-        assertScript '''
+        shouldFail MissingPropertyException, '''
             @Grab('commons-digester:commons-digester:2.1;transitive=false')
             import org.apache.commons.digester.Digester
 
             assert Digester.name.size() == 36
-            try {
-                assert org.apache.commons.beanutils.BeanUtils.name.size() == 38
-                assert false
-            } catch(MissingPropertyException mpe) { }
+            assert org.apache.commons.beanutils.BeanUtils.name.size() == 38
         '''
     }
 
+    @Test
     void testAutoDownloadGrapeConfig() {
-
         assertScript '''
             @Grab('commons-digester:commons-digester:2.1;transitive=false')
             import org.apache.commons.digester.Digester
 
             assert Digester.name.size() == 36
         '''
-
-        assert Grape.getInstance().ivyInstance.settings.defaultResolver.name == 'downloadGrapes'
+        assert Grape.instance.ivyInstance.settings.defaultResolver.name == 'downloadGrapes'
 
         assertScript '''
             @Grab('commons-digester:commons-digester:2.1;transitive=false')
@@ -336,8 +347,7 @@ class GrapeIvyTest extends CompilableTestSupport {
 
             assert Digester.name.size() == 36
         '''
-
-        assert Grape.getInstance().ivyInstance.settings.defaultResolver.name == 'cachedGrapes'
+        assert Grape.instance.ivyInstance.settings.defaultResolver.name == 'cachedGrapes'
 
         assertScript '''
             @Grab('commons-digester:commons-digester:2.1;transitive=false')
@@ -346,21 +356,18 @@ class GrapeIvyTest extends CompilableTestSupport {
 
             assert Digester.name.size() == 36
         '''
-
-        assert Grape.getInstance().ivyInstance.settings.defaultResolver.name == 'downloadGrapes'
+        assert Grape.instance.ivyInstance.settings.defaultResolver.name == 'downloadGrapes'
     }
 
-    /**
-     * GROOVY-470: multiple jars should be loaded for an artifacts with and without a classifier
-     */
+    @Test // GROOVY-470: multiple jars should be loaded for an artifacts with and without a classifier
     void testClassifierAndNonClassifierOnSameArtifact() {
         GroovyClassLoader loader = new GroovyClassLoader()
         Grape.grab(groupId:'org.neo4j', artifactId:'neo4j-kernel', version:'2.0.0-RC1', classLoader:loader)
         Grape.grab(groupId:'org.neo4j', artifactId:'neo4j-kernel', version:'2.0.0-RC1', classifier:'tests', classLoader:loader)
 
         def jars = jarNames(loader)
-        assert jars.contains ("neo4j-kernel-2.0.0-RC1.jar")
-        assert jars.contains ("neo4j-kernel-2.0.0-RC1-tests.jar")
+        assert jars.contains('neo4j-kernel-2.0.0-RC1.jar')
+        assert jars.contains('neo4j-kernel-2.0.0-RC1-tests.jar')
 
         // also check reverse ordering of deps
         loader = new GroovyClassLoader()
@@ -368,27 +375,33 @@ class GrapeIvyTest extends CompilableTestSupport {
         Grape.grab(groupId:'org.neo4j', artifactId:'neo4j-kernel', version:'2.0.0-RC1', classLoader:loader)
 
         jars = jarNames(loader)
-        assert jars.contains ("neo4j-kernel-2.0.0-RC1.jar")
-        assert jars.contains ("neo4j-kernel-2.0.0-RC1-tests.jar")
+        assert jars.contains('neo4j-kernel-2.0.0-RC1.jar')
+        assert jars.contains('neo4j-kernel-2.0.0-RC1-tests.jar')
     }
 
-    void testSystemProperties_groovy7548() {
+    @Test // GROOVY-7548
+    void testSystemProperties() {
         System.setProperty('groovy7548prop', 'x')
         assert System.getProperty('groovy7548prop') == 'x'
-        new GroovyShell().evaluate '''
-            @GrabConfig(systemProperties='groovy7548prop=y')
-            @Grab('commons-lang:commons-lang:2.6')
-            import org.apache.commons.lang.StringUtils
-            assert StringUtils.name == 'org.apache.commons.lang.StringUtils'
-        '''
-        assert System.getProperty('groovy7548prop') == 'y'
+        try {
+            assertScript '''
+                @GrabConfig(systemProperties='groovy7548prop=y')
+                @Grab('commons-lang:commons-lang:2.6')
+                import org.apache.commons.lang.StringUtils
+
+                assert StringUtils.name == 'org.apache.commons.lang.StringUtils'
+            '''
+            assert System.getProperty('groovy7548prop') == 'y'
+        } finally {
+            System.clearProperty('groovy7548prop')
+        }
     }
 
-    // GROOVY-7649
+    @Test // GROOVY-7649
     void testResolveSucceedsAfterFailure() {
         GroovyClassLoader loader = new GroovyClassLoader()
 
-        shouldFail{
+        shouldFail {
             Grape.resolve([classLoader:loader], [], [groupId:'bogus', artifactId:'bogus', version:'0.1'])
         }