Re: ant git commit: Inline buildfile names, make search easier

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Jaikiran Pai
What purpose is this change serving?

-Jaikiran


On 30/04/18 10:08 AM, [hidden email] wrote:

> Repository: ant
> Updated Branches:
>    refs/heads/master 0add85310 -> f3dfb7779
>
>
> Inline buildfile names, make search easier
>
> Project: http://git-wip-us.apache.org/repos/asf/ant/repo
> Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/f3dfb777
> Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/f3dfb777
> Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/f3dfb777
>
> Branch: refs/heads/master
> Commit: f3dfb7779a528439a390ab09383cd8cb6b7e8307
> Parents: 0add853
> Author: Gintas Grigelionis <[hidden email]>
> Authored: Mon Apr 30 06:38:27 2018 +0200
> Committer: Gintas Grigelionis <[hidden email]>
> Committed: Mon Apr 30 06:38:27 2018 +0200
>
> ----------------------------------------------------------------------
>   .../apache/tools/ant/taskdefs/ExecTaskTest.java |  4 +---
>   .../apache/tools/ant/taskdefs/JavadocTest.java  |  6 +-----
>   .../apache/tools/ant/taskdefs/ParallelTest.java |  5 +----
>   .../tools/ant/taskdefs/PathConvertTest.java     |  4 +---
>   .../tools/ant/taskdefs/RmicAdvancedTest.java    |  4 +---
>   .../apache/tools/ant/taskdefs/SleepTest.java    |  3 +--
>   .../org/apache/tools/ant/taskdefs/WarTest.java  |  4 +---
>   .../tools/ant/taskdefs/optional/ANTLRTest.java  |  4 +---
>   .../taskdefs/optional/EchoPropertiesTest.java   | 21 ++++++--------------
>   .../tools/ant/taskdefs/optional/JavahTest.java  |  4 +---
>   .../tools/ant/taskdefs/optional/JspcTest.java   |  4 +---
>   .../ant/taskdefs/optional/Native2AsciiTest.java |  5 +----
>   .../taskdefs/optional/ReplaceRegExpTest.java    |  3 +--
>   .../taskdefs/optional/RhinoReferenceTest.java   |  3 +--
>   .../taskdefs/optional/SchemaValidateTest.java   |  7 +------
>   .../optional/XmlValidateCatalogTest.java        |  7 +------
>   .../ant/taskdefs/optional/XmlValidateTest.java  |  7 +------
>   .../taskdefs/optional/depend/DependTest.java    |  5 +----
>   .../ant/taskdefs/optional/image/ImageTest.java  |  3 +--
>   .../junit/XMLFormatterWithCDATAOnSystemOut.java |  3 +--
>   20 files changed, 25 insertions(+), 81 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/ExecTaskTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/ExecTaskTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/ExecTaskTest.java
> index fd92ded..cf30955 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/ExecTaskTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/ExecTaskTest.java
> @@ -44,8 +44,6 @@ public class ExecTaskTest {
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
> -    private static final String BUILD_PATH = "src/etc/testcases/taskdefs/exec/";
> -    private static final String BUILD_FILE = BUILD_PATH + "exec.xml";
>       private static final int TIME_TO_WAIT = 1;
>       /** maximum time allowed for the build in milliseconds */
>       private static final int MAX_BUILD_TIME = 6000;
> @@ -60,7 +58,7 @@ public class ExecTaskTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(BUILD_FILE);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/exec/exec.xml");
>       }
>  
>       @Test
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/JavadocTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/JavadocTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/JavadocTest.java
> index f7a287d..cae8620 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/JavadocTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/JavadocTest.java
> @@ -28,13 +28,9 @@ public class JavadocTest {
>       @Rule
>       public final BuildFileRule buildRule = new BuildFileRule();
>  
> -    private static final String BUILD_PATH = "src/etc/testcases/taskdefs/javadoc/";
> -    private static final String BUILD_FILENAME = "javadoc.xml";
> -    private static final String BUILD_FILE = BUILD_PATH + BUILD_FILENAME;
> -
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(BUILD_FILE);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/javadoc/javadoc.xml");
>       }
>  
>       // PR 38370
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/ParallelTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/ParallelTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/ParallelTest.java
> index eba49c9..c08564e 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/ParallelTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/ParallelTest.java
> @@ -52,14 +52,11 @@ public class ParallelTest {
>       /** Standard property value for the fail test */
>       public static final String FAILURE_MESSAGE = "failure";
>  
> -    /** the build file associated with this test */
> -    public static final String TEST_BUILD_FILE = "src/etc/testcases/taskdefs/parallel.xml";
> -
>       private Project p;
>       /** The JUnit setup method */
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TEST_BUILD_FILE);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/parallel.xml");
>           p = buildRule.getProject();
>       }
>  
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/PathConvertTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/PathConvertTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/PathConvertTest.java
> index f29dc12..8a706e6 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/PathConvertTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/PathConvertTest.java
> @@ -33,13 +33,11 @@ public class PathConvertTest {
>       @Rule
>       public final BuildFileRule buildRule = new BuildFileRule();
>  
> -    private static final String BUILD_PATH = "src/etc/testcases/taskdefs/";
>       private static final String BUILD_FILENAME = "pathconvert.xml";
> -    private static final String BUILD_FILE = BUILD_PATH + BUILD_FILENAME;
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(BUILD_FILE);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/" + BUILD_FILENAME);
>       }
>  
>       @Test
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java
> index f162b2f..2322a36 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java
> @@ -41,8 +41,6 @@ import static org.junit.Assume.assumeTrue;
>    */
>   public class RmicAdvancedTest {
>  
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/rmic/";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
> @@ -54,7 +52,7 @@ public class RmicAdvancedTest {
>        */
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "rmic.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/rmic/rmic.xml");
>       }
>  
>       /**
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/SleepTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/SleepTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/SleepTest.java
> index 163d485..a1853c5 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/SleepTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/SleepTest.java
> @@ -31,12 +31,11 @@ public class SleepTest {
>       @Rule
>       public final BuildFileRule buildRule = new BuildFileRule();
>  
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/";
>       private static final int ERROR_RANGE = 1000;
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "sleep.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/sleep.xml");
>       }
>  
>       @Test
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/WarTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/WarTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/WarTest.java
> index 1513ac7..80d0df6 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/WarTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/WarTest.java
> @@ -32,15 +32,13 @@ import static org.junit.Assert.assertTrue;
>    *
>    */
>   public class WarTest {
> -    public static final String TEST_BUILD_FILE
> -        = "src/etc/testcases/taskdefs/war.xml";
>  
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TEST_BUILD_FILE);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/war.xml");
>       }
>  
>       /**
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ANTLRTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ANTLRTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ANTLRTest.java
> index 895617f..efcbf01 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ANTLRTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ANTLRTest.java
> @@ -46,14 +46,12 @@ import static org.junit.Assert.assertThat;
>    */
>   public class ANTLRTest {
>  
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/antlr/";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "antlr.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/antlr/antlr.xml");
>       }
>  
>       /**
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/EchoPropertiesTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/EchoPropertiesTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/EchoPropertiesTest.java
> index bc298f8..f31ff46 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/EchoPropertiesTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/EchoPropertiesTest.java
> @@ -20,6 +20,7 @@ package org.apache.tools.ant.taskdefs.optional;
>  
>   import static org.hamcrest.Matchers.containsString;
>   import static org.junit.Assert.assertEquals;
> +import static org.junit.Assert.assertNotNull;
>   import static org.junit.Assert.assertNull;
>   import static org.junit.Assert.assertThat;
>   import static org.junit.Assert.assertTrue;
> @@ -51,7 +52,6 @@ import org.junit.rules.ExpectedException;
>    */
>   public class EchoPropertiesTest {
>  
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/";
>       private static final String GOOD_OUTFILE = "test.properties";
>       private static final String GOOD_OUTFILE_XML = "test.xml";
>       private static final String PREFIX_OUTFILE = "test-prefix.properties";
> @@ -65,7 +65,7 @@ public class EchoPropertiesTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "echoproperties.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/echoproperties.xml");
>           buildRule.getProject().setProperty("test.property", TEST_VALUE);
>       }
>  
> @@ -123,7 +123,7 @@ public class EchoPropertiesTest {
>           buildRule.executeTarget("testEchoToGoodFileXml");
>  
>           // read in the file
> -        File f = createRelativeFile(GOOD_OUTFILE_XML);
> +        File f = new File(buildRule.getProject().getBaseDir(), GOOD_OUTFILE_XML);
>           try (BufferedReader br = new BufferedReader(new FileReader(f))) {
>               assertTrue("did not encounter set property in generated file.", br.lines().anyMatch(line
>                       -> line.contains("<property name=\"test.property\" value=\"" + TEST_VALUE + "\" />")));
> @@ -196,7 +196,9 @@ public class EchoPropertiesTest {
>  
>       protected Properties loadPropFile(String relativeFilename)
>               throws IOException {
> -        File f = createRelativeFile(relativeFilename);
> +        assertNotNull("Null property file name", relativeFilename);
> +        File f = new File(buildRule.getProject().getBaseDir(), relativeFilename);
> +        assertTrue("Did not create " + f.getAbsolutePath(), f.exists());
>           Properties props = new Properties();
>           try (InputStream in = new BufferedInputStream(new FileInputStream(f))) {
>               props.load(in);
> @@ -205,21 +207,10 @@ public class EchoPropertiesTest {
>       }
>  
>       protected void assertGoodFile() throws Exception {
> -        File f = createRelativeFile(GOOD_OUTFILE);
> -        assertTrue("Did not create " + f.getAbsolutePath(),
> -            f.exists());
>           Properties props = loadPropFile(GOOD_OUTFILE);
>           props.list(System.out);
>           assertEquals("test property not found ",
>                        TEST_VALUE, props.getProperty("test.property"));
>       }
>  
> -    protected String toAbsolute(String filename) {
> -        return createRelativeFile(filename).getAbsolutePath();
> -    }
> -
> -    protected File createRelativeFile(String filename) {
> -        return filename.equals(".") ? buildRule.getProject().getBaseDir()
> -                : new File(buildRule.getProject().getBaseDir(), filename);
> -    }
>   }
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JavahTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JavahTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JavahTest.java
> index d58c080..edff2c0 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JavahTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JavahTest.java
> @@ -31,14 +31,12 @@ import static org.junit.Assume.assumeFalse;
>  
>   public class JavahTest {
>  
> -    private static final String BUILD_XML = "src/etc/testcases/taskdefs/optional/javah/build.xml";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(BUILD_XML);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/javah/build.xml");
>       }
>  
>       @After
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JspcTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JspcTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JspcTest.java
> index d4cdda9..436fd65 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JspcTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/JspcTest.java
> @@ -47,8 +47,6 @@ import static org.junit.Assert.assertTrue;
>    */
>   public class JspcTest {
>  
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/";
> -
>       @Rule
>       public ExpectedException thrown = ExpectedException.none();
>  
> @@ -57,7 +55,7 @@ public class JspcTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "jspc.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/jspc.xml");
>        }
>  
>       @Test
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/Native2AsciiTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/Native2AsciiTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/Native2AsciiTest.java
> index 0ea5d9b..a4a1890 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/Native2AsciiTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/Native2AsciiTest.java
> @@ -32,15 +32,12 @@ import static org.junit.Assert.assertTrue;
>  
>   public class Native2AsciiTest {
>  
> -    private static final String BUILD_XML =
> -        "src/etc/testcases/taskdefs/optional/native2ascii/build.xml";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(BUILD_XML);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/native2ascii/build.xml");
>       }
>  
>       @After
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ReplaceRegExpTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ReplaceRegExpTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ReplaceRegExpTest.java
> index cbf278a..c214ce4 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ReplaceRegExpTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/ReplaceRegExpTest.java
> @@ -39,14 +39,13 @@ import static org.junit.Assume.assumeTrue;
>    *
>    */
>   public class ReplaceRegExpTest {
> -    private static final String PROJECT_PATH = "src/etc/testcases/taskdefs/optional";
>  
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(PROJECT_PATH + "/replaceregexp.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/replaceregexp.xml");
>       }
>  
>       @Test
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/RhinoReferenceTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/RhinoReferenceTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/RhinoReferenceTest.java
> index 41803de..34e5401 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/RhinoReferenceTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/RhinoReferenceTest.java
> @@ -34,8 +34,7 @@ public class RhinoReferenceTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(
> -                "src/etc/testcases/taskdefs/optional/script_reference.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/script_reference.xml");
>       }
>  
>       @Test
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/SchemaValidateTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/SchemaValidateTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/SchemaValidateTest.java
> index 1cb0451..c621e0e 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/SchemaValidateTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/SchemaValidateTest.java
> @@ -30,11 +30,6 @@ import org.junit.rules.ExpectedException;
>  
>   public class SchemaValidateTest {
>  
> -    /**
> -     * where tasks run
> -     */
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
> @@ -43,7 +38,7 @@ public class SchemaValidateTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "schemavalidate.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/schemavalidate.xml");
>       }
>  
>       /**
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateCatalogTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateCatalogTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateCatalogTest.java
> index 33fa189..8f3caa4 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateCatalogTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateCatalogTest.java
> @@ -30,17 +30,12 @@ import org.junit.Test;
>    */
>   public class XmlValidateCatalogTest {
>  
> -    /**
> -     * where tasks run
> -     */
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "xmlvalidate.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/xmlvalidate.xml");
>       }
>  
>  
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateTest.java
> index 30b2a00..6c97422 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/XmlValidateTest.java
> @@ -34,11 +34,6 @@ import org.junit.rules.ExpectedException;
>    */
>   public class XmlValidateTest {
>  
> -    /**
> -     * where tasks run
> -     */
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
> @@ -47,7 +42,7 @@ public class XmlValidateTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TASKDEFS_DIR + "xmlvalidate.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/xmlvalidate.xml");
>       }
>  
>       /**
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/depend/DependTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/depend/DependTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/depend/DependTest.java
> index d97afcc..023c36b 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/depend/DependTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/depend/DependTest.java
> @@ -47,9 +47,6 @@ import static org.junit.Assert.assertTrue;
>   public class DependTest {
>       public static final String RESULT_FILESET = "result";
>  
> -    public static final String TEST_BUILD_FILE
> -        = "src/etc/testcases/taskdefs/optional/depend/depend.xml";
> -
>       @Rule
>       public BuildFileRule buildRule = new BuildFileRule();
>  
> @@ -58,7 +55,7 @@ public class DependTest {
>  
>       @Before
>       public void setUp() {
> -        buildRule.configureProject(TEST_BUILD_FILE);
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/depend/depend.xml");
>       }
>  
>       /**
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java
> index 3fda82c..2d7f43c 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java
> @@ -44,7 +44,6 @@ import static org.junit.Assume.assumeTrue;
>   public class ImageTest {
>  
>       private static final FileUtils FILE_UTILS = FileUtils.getFileUtils();
> -    private static final String TASKDEFS_DIR = "src/etc/testcases/taskdefs/optional/image/";
>       private static final String LARGEIMAGE = "largeimage.jpg";
>  
>       @Rule
> @@ -57,7 +56,7 @@ public class ImageTest {
>       public void setUp() {
>           /* JAI depends on internal API removed in Java 9 */
>           assumeFalse(JavaEnvUtils.isAtLeastJavaVersion(JavaEnvUtils.JAVA_9));
> -        buildRule.configureProject(TASKDEFS_DIR + "image.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/image/image.xml");
>       }
>  
>  
>
> http://git-wip-us.apache.org/repos/asf/ant/blob/f3dfb777/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junit/XMLFormatterWithCDATAOnSystemOut.java
> ----------------------------------------------------------------------
> diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junit/XMLFormatterWithCDATAOnSystemOut.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junit/XMLFormatterWithCDATAOnSystemOut.java
> index 64674a2..55aed16 100644
> --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junit/XMLFormatterWithCDATAOnSystemOut.java
> +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junit/XMLFormatterWithCDATAOnSystemOut.java
> @@ -31,7 +31,6 @@ import static org.junit.Assert.assertThat;
>  
>   public class XMLFormatterWithCDATAOnSystemOut {
>  
> -    private static final String DIR = "src/etc/testcases/taskdefs/optional/junit";
>       private static final String REPORT =
>           "TEST-" + XMLFormatterWithCDATAOnSystemOut.class.getName() + ".xml";
>  
> @@ -61,7 +60,7 @@ public class XMLFormatterWithCDATAOnSystemOut {
>  
>       @Test
>       public void testBuildfile() throws IOException {
> -        buildRule.configureProject(DIR + "/cdataoutput.xml");
> +        buildRule.configureProject("src/etc/testcases/taskdefs/optional/junit/cdataoutput.xml");
>           if (buildRule.getProject().getProperty("cdata.inner") == null) {
>               // avoid endless loop
>               buildRule.executeTarget("run-junit");
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
Names of buildfiles used in tests can be found by "grep configureProject"?
The point is that these names are used exactly once, and need not to be put
in a field which is named inconsistently.
Deviations from this rule of thumb indicate that tests are parameterized in
a manner where each test has its own buildfile.

Gintas

2018-04-30 4:43 GMT+00:00 Jaikiran Pai <[hidden email]>:

> What purpose is this change serving?
>
> -Jaikiran
>
>
> On 30/04/18 10:08 AM, [hidden email] wrote:
>
>> Repository: ant
>> Updated Branches:
>>    refs/heads/master 0add85310 -> f3dfb7779
>>
>>
>> Inline buildfile names, make search easier
>>
>> Project: http://git-wip-us.apache.org/repos/asf/ant/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/f3dfb777
>> Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/f3dfb777
>> Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/f3dfb777
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Jaikiran Pai

On 30/04/18 11:12 AM, Gintautas Grigelionis wrote:
> Names of buildfiles used in tests can be found by "grep configureProject"?
Why is that even a requirement?

I think I am just repeating myself again and again in different mails.
Changes like these are random personal preferences. The fact that these
are being done even after the mail discussions we recently had,
indicates that the request to not do such changes have been ignored.
This is going down the route of a being some kind of a personal
individual hobby project. So I'm just going to stop bothering looking
into these commits anymore and just stay away from them and stop sending
this frustrated and rude sounding mails.

-Jaikiran

> The point is that these names are used exactly once, and need not to be put
> in a field which is named inconsistently.
> Deviations from this rule of thumb indicate that tests are parameterized in
> a manner where each test has its own buildfile.
>
> Gintas
>
> 2018-04-30 4:43 GMT+00:00 Jaikiran Pai <[hidden email]>:
>
>> What purpose is this change serving?
>>
>> -Jaikiran
>>
>>
>> On 30/04/18 10:08 AM, [hidden email] wrote:
>>
>>> Repository: ant
>>> Updated Branches:
>>>     refs/heads/master 0add85310 -> f3dfb7779
>>>
>>>
>>> Inline buildfile names, make search easier
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/ant/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/f3dfb777
>>> Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/f3dfb777
>>> Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/f3dfb777
>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
My apologies for offending anyone; just one last silly question: why
uniformity is not a requirement?
I believe that even one language that espoused TMTOWDI has moved to
TIMTOWTDIBSCINABTE?

Gintas

2018-04-30 5:56 GMT+00:00 Jaikiran Pai <[hidden email]>:

>
> On 30/04/18 11:12 AM, Gintautas Grigelionis wrote:
>
>> Names of buildfiles used in tests can be found by "grep configureProject"?
>>
> Why is that even a requirement?
>
> I think I am just repeating myself again and again in different mails.
> Changes like these are random personal preferences. The fact that these are
> being done even after the mail discussions we recently had, indicates that
> the request to not do such changes have been ignored. This is going down
> the route of a being some kind of a personal individual hobby project. So
> I'm just going to stop bothering looking into these commits anymore and
> just stay away from them and stop sending this frustrated and rude sounding
> mails.
>
> -Jaikiran
>
> The point is that these names are used exactly once, and need not to be put
>> in a field which is named inconsistently.
>> Deviations from this rule of thumb indicate that tests are parameterized
>> in
>> a manner where each test has its own buildfile.
>>
>> Gintas
>>
>> 2018-04-30 4:43 GMT+00:00 Jaikiran Pai <[hidden email]>:
>>
>> What purpose is this change serving?
>>>
>>> -Jaikiran
>>>
>>>
>>> On 30/04/18 10:08 AM, [hidden email] wrote:
>>>
>>> Repository: ant
>>>> Updated Branches:
>>>>     refs/heads/master 0add85310 -> f3dfb7779
>>>>
>>>>
>>>> Inline buildfile names, make search easier
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/ant/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/f3dfb777
>>>> Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/f3dfb777
>>>> Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/f3dfb777
>>>>
>>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Stefan Bodewig
On 2018-04-30, Gintautas Grigelionis wrote:

> My apologies for offending anyone; just one last silly question: why
> uniformity is not a requirement?

Who's uniformity do you pick? There are so many choices that only depend
on taste.

assertEquals(x, y) vs assertThat(y, equalTo(x)) amd many many small
nuances that we all don't need to agree on, as long as we understand
what the code means and does and we accept to not change code just
because it doesn't conform to our own choices.

So far the code used to be uniform enough for all other developers of
the code base.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Stefan Bodewig
In reply to this post by Jaikiran Pai
On 2018-04-30, Jaikiran Pai wrote:

> Changes like these are random personal preferences.

Right.

> The fact that these are being done even after the mail discussions we
> recently had, indicates that the request to not do such changes have
> been ignored. This is going down the route of a being some kind of a
> personal individual hobby project. So I'm just going to stop bothering
> looking into these commits anymore and just stay away from them and
> stop sending this frustrated and rude sounding mails.

I must admit I share your frustration and have to thank you for voicing
it when I didn't dare to do.

TBH I've given up on giving the changes to tests more that a cursory
look and hoping "this one is going to be the last one".

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
In reply to this post by Stefan Bodewig
2018-04-30 9:55 GMT+00:00 Stefan Bodewig <[hidden email]>:

> On 2018-04-30, Gintautas Grigelionis wrote:
>
> > My apologies for offending anyone; just one last silly question: why
> > uniformity is not a requirement?
>
> Who's uniformity do you pick? There are so many choices that only depend
> on taste.
>
> assertEquals(x, y) vs assertThat(y, equalTo(x)) amd many many small
> nuances that we all don't need to agree on, as long as we understand
> what the code means and does and we accept to not change code just
> because it doesn't conform to our own choices.
>

assertThat(object, matcher) is easier to parameterize, see MakeUrlTest.
And regarding parameterization, I have found at least three cases of
copy-paste errors
where two supposedly different test cases were, in fact, identical.

Gintas
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Jaikiran Pai

On 30/04/18 3:52 PM, Gintautas Grigelionis wrote:

> 2018-04-30 9:55 GMT+00:00 Stefan Bodewig <[hidden email]>:
>
>> On 2018-04-30, Gintautas Grigelionis wrote:
>>
>>> My apologies for offending anyone; just one last silly question: why
>>> uniformity is not a requirement?
>> Who's uniformity do you pick? There are so many choices that only depend
>> on taste.
>>
>> assertEquals(x, y) vs assertThat(y, equalTo(x)) amd many many small
>> nuances that we all don't need to agree on, as long as we understand
>> what the code means and does and we accept to not change code just
>> because it doesn't conform to our own choices.
>>
> assertThat(object, matcher) is easier to parameterize, see MakeUrlTest.
> And regarding parameterization, I have found at least three cases of
> copy-paste errors
> where two supposedly different test cases were, in fact, identical.
>
The assertEquals vs assertThat was an example of different ways a
certain thing can be implemented. I hope you do realize that what we are
discussing in this thread (and others) is much more broader than finding
copy/paste errors due to usage of one API over other.

-Jaikiran

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Nicolas Lalevée
In reply to this post by Stefan Bodewig


> Le 30 avr. 2018 à 11:58, Stefan Bodewig <[hidden email]> a écrit :
>
> On 2018-04-30, Jaikiran Pai wrote:
>
>> Changes like these are random personal preferences.
>
> Right.
>
>> The fact that these are being done even after the mail discussions we
>> recently had, indicates that the request to not do such changes have
>> been ignored. This is going down the route of a being some kind of a
>> personal individual hobby project. So I'm just going to stop bothering
>> looking into these commits anymore and just stay away from them and
>> stop sending this frustrated and rude sounding mails.
>
> I must admit I share your frustration and have to thank you for voicing
> it when I didn't dare to do.
>
> TBH I've given up on giving the changes to tests more that a cursory
> look and hoping "this one is going to be the last one".

Same here.

Nicolas


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Jaikiran Pai
In reply to this post by Gintautas Grigelionis

On 30/04/18 12:52 PM, Gintautas Grigelionis wrote:
> My apologies for offending anyone; just one last silly question: why
> uniformity is not a requirement?
I think it has already been explained in the other thread why it's not a
necessity for a project as large and as old as this one, especially when
the changes aren't solving anything. Plus, I believe you even agreed to
it, when this was explained in that other thread.

Either way, this isn't about uniformity. Take a look at this commit for
example (and it's just one of the many). It was done, as you admit,
because you wanted to find a particular piece of text when you run "grep
configureProject". There's no level of uniformity that can solve such
random requirements, unless of course the code is auto-generated to fit
in these requirements.

> I believe that even one language that espoused TMTOWDI has moved to
> TIMTOWTDIBSCINABTE?
Sorry, I really don't know what that means or how that relates to what
we are discussing.

-Jaikiran

>
> Gintas
>
> 2018-04-30 5:56 GMT+00:00 Jaikiran Pai <[hidden email]>:
>
>> On 30/04/18 11:12 AM, Gintautas Grigelionis wrote:
>>
>>> Names of buildfiles used in tests can be found by "grep configureProject"?
>>>
>> Why is that even a requirement?
>>
>> I think I am just repeating myself again and again in different mails.
>> Changes like these are random personal preferences. The fact that these are
>> being done even after the mail discussions we recently had, indicates that
>> the request to not do such changes have been ignored. This is going down
>> the route of a being some kind of a personal individual hobby project. So
>> I'm just going to stop bothering looking into these commits anymore and
>> just stay away from them and stop sending this frustrated and rude sounding
>> mails.
>>
>> -Jaikiran
>>
>> The point is that these names are used exactly once, and need not to be put
>>> in a field which is named inconsistently.
>>> Deviations from this rule of thumb indicate that tests are parameterized
>>> in
>>> a manner where each test has its own buildfile.
>>>
>>> Gintas
>>>
>>> 2018-04-30 4:43 GMT+00:00 Jaikiran Pai <[hidden email]>:
>>>
>>> What purpose is this change serving?
>>>> -Jaikiran
>>>>
>>>>
>>>> On 30/04/18 10:08 AM, [hidden email] wrote:
>>>>
>>>> Repository: ant
>>>>> Updated Branches:
>>>>>      refs/heads/master 0add85310 -> f3dfb7779
>>>>>
>>>>>
>>>>> Inline buildfile names, make search easier
>>>>>
>>>>> Project: http://git-wip-us.apache.org/repos/asf/ant/repo
>>>>> Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/f3dfb777
>>>>> Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/f3dfb777
>>>>> Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/f3dfb777
>>>>>
>>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

AW: ant git commit: Inline buildfile names, make search easier

Jan Matèrne (jhm)
In reply to this post by Stefan Bodewig
> > Changes like these are random personal preferences.
>
> Right.
>
> > The fact that these are being done even after the mail discussions we
> > recently had, indicates that the request to not do such changes have
> > been ignored. This is going down the route of a being some kind of a
> > personal individual hobby project. So I'm just going to stop
> bothering
> > looking into these commits anymore and just stay away from them and
> > stop sending this frustrated and rude sounding mails.
>
> I must admit I share your frustration and have to thank you for voicing
> it when I didn't dare to do.
>
> TBH I've given up on giving the changes to tests more that a cursory
> look and hoping "this one is going to be the last one".


In my eyes most of the committers don't like these "code style" changes.
So please stop these changes and invest your energy more productive.

I don't think it would be helpful if another committer starts reverting the
changes because of a different preference ...


Jan




---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
In reply to this post by Jaikiran Pai
2018-04-30 10:35 GMT+00:00 Jaikiran Pai <[hidden email]>:

>
> On 30/04/18 3:52 PM, Gintautas Grigelionis wrote:
>
>> 2018-04-30 9:55 GMT+00:00 Stefan Bodewig <[hidden email]>:
>>
>> On 2018-04-30, Gintautas Grigelionis wrote:
>>>
>>> My apologies for offending anyone; just one last silly question: why
>>>> uniformity is not a requirement?
>>>>
>>> Who's uniformity do you pick? There are so many choices that only depend
>>> on taste.
>>>
>>> assertEquals(x, y) vs assertThat(y, equalTo(x)) amd many many small
>>> nuances that we all don't need to agree on, as long as we understand
>>> what the code means and does and we accept to not change code just
>>> because it doesn't conform to our own choices.
>>>
>>> assertThat(object, matcher) is easier to parameterize, see MakeUrlTest.
>> And regarding parameterization, I have found at least three cases of
>> copy-paste errors
>> where two supposedly different test cases were, in fact, identical.
>>
>> The assertEquals vs assertThat was an example of different ways a certain
> thing can be implemented. I hope you do realize that what we are discussing
> in this thread (and others) is much more broader than finding copy/paste
> errors due to usage of one API over other.


I do realize that some changes might be a subject for discussion; the
overarching goal, however, is to reduce verbosity, because verbosity makes
it easier to hide mistakes.
If you believe that you might achieve that by another choice, please
provide a relevant example.

Gintas
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
In reply to this post by Jaikiran Pai
2018-04-30 10:43 GMT+00:00 Jaikiran Pai <[hidden email]>:

>
> On 30/04/18 12:52 PM, Gintautas Grigelionis wrote:
>
>> My apologies for offending anyone; just one last silly question: why
>> uniformity is not a requirement?
>>
> I think it has already been explained in the other thread why it's not a
> necessity for a project as large and as old as this one, especially when
> the changes aren't solving anything. Plus, I believe you even agreed to it,
> when this was explained in that other thread.
>
> Either way, this isn't about uniformity. Take a look at this commit for
> example (and it's just one of the many). It was done, as you admit, because
> you wanted to find a particular piece of text when you run "grep
> configureProject". There's no level of uniformity that can solve such
> random requirements, unless of course the code is auto-generated to fit in
> these requirements.
>

You're stuck on particulars, grep is just one tool. What I meant is
"searching the code to find relevant information easily", in this case, the
names of buildfiles used in setting up test cases.

Gintas
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
In reply to this post by Jan Matèrne (jhm)
2018-04-30 10:45 GMT+00:00 Jan Matèrne (jhm) <[hidden email]>:

> In my eyes most of the committers don't like these "code style" changes.
> So please stop these changes and invest your energy more productive.
>
> I don't think it would be helpful if another committer starts reverting the
> changes because of a different preference ...


With all due respect, please let me know what do you prefer: for instance,
is test parametrization good, bad or annoying?

Gintas
Reply | Threaded
Open this post in threaded view
|

AW: ant git commit: Inline buildfile names, make search easier

Jan Matèrne (jhm)
> > In my eyes most of the committers don't like these "code style"
> changes.
> > So please stop these changes and invest your energy more productive.
> >
> > I don't think it would be helpful if another committer starts
> > reverting the changes because of a different preference ...
>
>
> With all due respect, please let me know what do you prefer: for
> instance, is test parametrization good, bad or annoying?


For that single point I think parametrization is 'good'.
If I count right, it impacted three test classes.
Why change all the others?

Starting a discussion before such big changes would also be helpful.

Jan





---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
2018-04-30 11:20 GMT+00:00 Jan Matèrne (jhm) <[hidden email]>:

> For that single point I think parametrization is 'good'.
> If I count right, it impacted three test classes.
> Why change all the others?
>
> Starting a discussion before such big changes would also be helpful.
>

Just to pick a nit, there are 9 parameterized test classes.
I could easily add another 10-20 classes.
But, I'd rather put trivial changes into a separate commit
(I remember being told off for mixing in whitespace changes on previous
occasions...)

Anyway, I also recall that we had a discussion about unit tests which ended
up in decision on one detail:
any setup details (eg special properties) that can break tests and are
controlled by end user must be asserted rather than assumed.
The latter should be used for OS- and/or runtime-specific properties.

Gintas
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Stefan Bodewig
In reply to this post by Gintautas Grigelionis
On 2018-04-30, Gintautas Grigelionis wrote:

> I do realize that some changes might be a subject for discussion;

Gintas, please try to see what the others are saying. Most of the
changes are changes that nobody of those who've spoken up consider
improvements at all - apart from yourself. The outcome of the discussion
is pretty clear and this includes changes you don't even consider
subject for discussion.

> the overarching goal, however, is to reduce verbosity, because
> verbosity makes it easier to hide mistakes.

This is your goal and we should have decided together whether we agree
on this goal. The answer seems to be that we don't.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Inline buildfile names, make search easier

Gintautas Grigelionis
2018-04-30 13:13 GMT+00:00 Stefan Bodewig <[hidden email]>:

> On 2018-04-30, Gintautas Grigelionis wrote:
> > the overarching goal, however, is to reduce verbosity, because
> > verbosity makes it easier to hide mistakes.
>
> This is your goal and we should have decided together whether we agree
> on this goal. The answer seems to be that we don't.


I get that. What would be the answer if we reduce the scope to
parameterization of unit tests?

Gintas
Reply | Threaded
Open this post in threaded view
|

Parameterized Tests (was Re: ant git commit: Inline buildfile names, make search easier)

Stefan Bodewig
On 2018-04-30, Gintautas Grigelionis wrote:

> 2018-04-30 13:13 GMT+00:00 Stefan Bodewig <[hidden email]>:

>> On 2018-04-30, Gintautas Grigelionis wrote:
>>> the overarching goal, however, is to reduce verbosity, because
>>> verbosity makes it easier to hide mistakes.

>> This is your goal and we should have decided together whether we agree
>> on this goal. The answer seems to be that we don't.

> I get that. What would be the answer if we reduce the scope to
> parameterization of unit tests?

For new tests I'm all for using parameterized test. The same is true
when you add functionality and need to touch the test anyway.

I'm still not sure I understand which benefit you see by retrofitting
tests that have been written before @Parameterized was invented. They do
contain way too many asserts in a single test method, but all of them
pass, so this is somewhat moot as long as neither the test nor the code
under test is touched :-).

This boils down to "make cosmetic changes only when you need to touch
the code anyway", I guess.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Parameterized Tests (was Re: ant git commit: Inline buildfile names, make search easier)

Gintautas Grigelionis
2018-05-03 11:06 GMT+02:00 Stefan Bodewig <[hidden email]>:
>
> I'm still not sure I understand which benefit you see by retrofitting
> tests that have been written before @Parameterized was invented. They do
> contain way too many asserts in a single test method, but all of them
> pass, so this is somewhat moot as long as neither the test nor the code
> under test is touched :-).
>

The benefit is clarity of what is being tested. This perhaps pertains even
more to extraction of fixtures which lead to splitting up of test cases.
Regarding @Parameterized, it is said that it's easier to understand complex
data than complex code.
It's also easier to add new test cases should the need arise, less chance
of missed annotation or copy-pasting a target name.
Call it code churning, but churning actually separates butter from
buttermilk :-)

Gintas
12