Merge pull request #15 from taskcluster/bug1492543

Bug 1492543 - Treat leading/trailing slashes equivalently between implementations

Author: petemoore

pom.xml

@@ -153,7 +153,7 @@
       <testResource>
         <directory>.</directory>
         <includes>
-          <include>specification.yml</include>
+          <include>tests.yml</include>
         </includes>
       </testResource>
     </testResources>

specification.yml

@@ -2,8 +2,8 @@ const assert = require('assert');
 
 const TASKCLUSTER_NET = 'https://taskcluster.net';
 
-const cleanRoot = rootUrl => rootUrl.replace(/\/$/, '');
-const cleanPath = path => path.replace(/^\//, '');
+const cleanRoot = rootUrl => rootUrl.replace(/\/*$/, '');
+const cleanPath = path => path.replace(/^\/*/, '');
 
 class LegacyUrls {
   /**
@@ -61,7 +61,6 @@ class LegacyUrls {
 
 class Urls {
   constructor(rootUrl) {
-    assert(rootUrl, 'no rootUrl provided');
     this.rootUrl = cleanRoot(rootUrl);
   }
 

src/index.js

@@ -6,16 +6,10 @@ private Clean() {
     }
 
     public static String path(String path) {
-        if (path.startsWith("/")) {
-            return path.substring(1);
-        }
-        return path;
+        return path.replaceAll("^/*", "");
     }
 
     public static String url(String url) {
-        if (url.endsWith("/")) {
-            return url.substring(0, url.length() - 1);
-        }
-        return url;
+        return url.replaceAll("/*$", "");
     }
 }

src/main/java/org/mozilla/taskcluster/urls/Clean.java

@@ -1,11 +1,11 @@
 package org.mozilla.taskcluster.urls;
 
+import java.util.Map;
 import lombok.Data;
 
 @Data
 public class TestCase {
-    private String     type;
-    private String     expectedUrl;
-    private String     oldExpectedUrl;
-    private String[][] argSets;
+    private String              function;
+    private Map<String, String> expected;
+    private String[][]          argSets;
 }

src/test/java/org/mozilla/taskcluster/urls/TestCase.java

@@ -0,0 +1,10 @@
+package org.mozilla.taskcluster.urls;
+
+import java.util.Map;
+import lombok.Data;
+
+@Data
+public class TestsSpecification {
+    private TestCase[]            tests;
+    private Map<String, String[]> rootURLs;
+}

src/test/java/org/mozilla/taskcluster/urls/TestCases.java

@@ -2,6 +2,7 @@
 
 import java.io.InputStream;
 import java.io.IOException;
+import java.util.Map;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -11,40 +12,29 @@
 
 public class URLsTest {
 
-    private final static String[] oldRootURLs = new String[] { "https://taskcluster.net", "https://taskcluster.net/" };
-
-    private final static String[] newRootURLs = new String[] { "https://taskcluster.example.com",
-            "https://taskcluster.example.com/" };
-
     /**
      * greenTick is an ANSI byte sequence to render a light green tick in a
      * color console.
      */
-    private final static byte[]   greenTick   = new byte[] { 0x1b, 0x5b, 0x33, 0x32, 0x6d, (byte) 0xe2, (byte) 0x9c,
+    private final static byte[] greenTick = new byte[] { 0x1b, 0x5b, 0x33, 0x32, 0x6d, (byte) 0xe2, (byte) 0x9c,
             (byte) 0x93, 0x1b, 0x5b, 0x30, 0x6d };
 
     /**
      * testURLs iterates through the language-agnostic test cases defined in
-     * /specification.yml to ensure that the java implementation returns
-     * consistent results with the other language implementations.
+     * /tests.yml to ensure that the java implementation returns consistent
+     * results with the other language implementations.
      */
     @Test
     public void testURLs() throws Exception {
-        Yaml yaml = new Yaml(new Constructor(TestCases.class));
-        InputStream inputStream = this.getClass().getClassLoader().getResourceAsStream("specification.yml");
-        TestCases testCases = yaml.load(inputStream);
-        for (TestCase testCase : testCases.getSpecs()) {
-            for (String[] argSet : testCase.getArgSets()) {
-                String methodName = testCase.getType();
-
-                String oldExpectedUrl = testCase.getOldExpectedUrl();
-                for (String rootURL : oldRootURLs) {
-                    this.test(oldExpectedUrl, rootURL, methodName, argSet);
-                }
-
-                String newExpectedUrl = testCase.getExpectedUrl();
-                for (String rootURL : newRootURLs) {
-                    this.test(newExpectedUrl, rootURL, methodName, argSet);
+        Yaml yaml = new Yaml(new Constructor(TestsSpecification.class));
+        InputStream inputStream = this.getClass().getClassLoader().getResourceAsStream("tests.yml");
+        TestsSpecification spec = yaml.load(inputStream);
+        for (TestCase test : spec.getTests()) {
+            for (String[] argSet : test.getArgSets()) {
+                for (String cluster : spec.getRootURLs().keySet()) {
+                    for (String rootURL : spec.getRootURLs().get(cluster)) {
+                        this.test(test.getExpected().get(cluster), rootURL, test.getFunction(), argSet);
+                    }
                 }
             }
         }
@@ -57,9 +47,9 @@ private static void test(String expectedUrl, String rootURL, String methodName,
         System.out.println(" " + methodName + "(" + quotedList(rootURL, argSet) + ") = " + expectedUrl);
     }
 
-    public static String testFunc(String functionType, URLProvider urlProvider, String[] args)
+    public static String testFunc(String function, URLProvider urlProvider, String[] args)
             throws NoSuchMethodException {
-        switch (functionType) {
+        switch (function) {
         case "api":
             return urlProvider.api(args[0], args[1], args[2]);
         case "apiReference":
@@ -75,7 +65,7 @@ public static String testFunc(String functionType, URLProvider urlProvider, Stri
         case "servicesManifest":
             return urlProvider.servicesManifest();
         default:
-            throw new NoSuchMethodException("Unknown function type: " + functionType);
+            throw new NoSuchMethodException("Unknown function type: " + function);
         }
     }
 

src/test/java/org/mozilla/taskcluster/urls/TestsSpecification.java

@@ -1,110 +1,86 @@
 package tcurls
 
 import (
-	"fmt"
 	"io/ioutil"
 	"strings"
 	"testing"
 
 	"gopkg.in/yaml.v2"
 )
 
-const rootURL = "https://taskcluster.example.com"
-
-type spec struct {
-	FunctionType   string     `yaml:"type"`
-	ExpectedURL    string     `yaml:"expectedUrl"`
-	OldExpectedURL string     `yaml:"oldExpectedUrl"`
-	ArgSets        [][]string `yaml:"argSets"`
+type TestCase struct {
+	Function string            `yaml:"function"`
+	Expected map[string]string `yaml:"expected"`
+	ArgSets  [][]string        `yaml:"argSets"`
 }
 
-type document struct {
-	Specs []spec `yaml:"specs"`
+type TestsSpecification struct {
+	RootURLs map[string][]string `yaml:"rootURLs"`
+	Tests    []TestCase          `yaml:"tests"`
 }
 
-func testFunc(t *testing.T, functionType string, root string, args ...string) (string, error) {
-	switch functionType {
+func testFunc(t *testing.T, function string, expectedURL string, root string, args ...string) {
+	var actualURL string
+	switch function {
 	case "api":
-		return API(root, args[0], args[1], args[2]), nil
+		actualURL = API(root, args[0], args[1], args[2])
 	case "apiReference":
-		return APIReference(root, args[0], args[1]), nil
+		actualURL = APIReference(root, args[0], args[1])
 	case "docs":
-		return Docs(root, args[0]), nil
+		actualURL = Docs(root, args[0])
 	case "exchangeReference":
-		return ExchangeReference(root, args[0], args[1]), nil
+		actualURL = ExchangeReference(root, args[0], args[1])
 	case "schema":
-		return Schema(root, args[0], args[1]), nil
+		actualURL = Schema(root, args[0], args[1])
 	case "ui":
-		return UI(root, args[0]), nil
+		actualURL = UI(root, args[0])
 	case "servicesManifest":
-		return ServicesManifest(root), nil
+		actualURL = ServicesManifest(root)
 	default:
-		return "", fmt.Errorf("Unknown function type: %s", functionType)
+		t.Errorf("Unknown function type: %s", function)
+		return
+	}
+	if expectedURL != actualURL {
+		t.Errorf("%v %v(%v) = `%v` but should be `%v`", redCross(), function, quotedList(root, args), actualURL, expectedURL)
+		return
 	}
+	t.Logf("%v %v(%v) = `%v`", greenTick(), function, quotedList(root, args), actualURL)
 }
 
 func TestURLs(t *testing.T) {
-	data, err := ioutil.ReadFile("specification.yml")
+	data, err := ioutil.ReadFile("tests.yml")
 	if err != nil {
 		t.Error(err)
 	}
-	var specs document
-	err = yaml.Unmarshal([]byte(data), &specs)
+	var spec TestsSpecification
+	err = yaml.Unmarshal([]byte(data), &spec)
 	if err != nil {
 		t.Error(err)
 	}
 
-	for _, test := range specs.Specs {
+	for _, test := range spec.Tests {
 		for _, argSet := range test.ArgSets {
-
-			// Test "new" URLs
-			result, err := testFunc(t, test.FunctionType, rootURL, argSet...)
-			if err != nil {
-				t.Error(err)
-				continue
-			}
-			if result != test.ExpectedURL {
-				t.Errorf("URL is not correct. Got %q wanted %q", result, test.ExpectedURL)
-				continue
+			for cluster, rootURLs := range spec.RootURLs {
+				for _, rootURL := range rootURLs {
+					testFunc(t, test.Function, test.Expected[cluster], rootURL, argSet...)
+				}
 			}
-			result, err = testFunc(t, test.FunctionType, fmt.Sprintf("%s/", rootURL), argSet...)
-			if err != nil {
-				t.Error(err)
-			}
-			if result != test.ExpectedURL {
-				t.Errorf("URL is not correct. Got %q wanted %q", result, test.ExpectedURL)
-				continue
-			}
-			t.Logf(`%v %v(%v) = %q`, greenTick(), test.FunctionType, quotedList(rootURL, argSet), result)
-
-			// Test "old" URLs
-			result, err = testFunc(t, test.FunctionType, oldRootURL, argSet...)
-			if err != nil {
-				t.Error(err)
-			}
-			if result != test.OldExpectedURL {
-				t.Errorf("URL is not correct. Got %q wanted %q", result, test.OldExpectedURL)
-			}
-			result, err = testFunc(t, test.FunctionType, fmt.Sprintf("%s/", oldRootURL), argSet...)
-			if err != nil {
-				t.Error(err)
-			}
-			if result != test.OldExpectedURL {
-				t.Errorf("URL is not correct. Got %q wanted %q", result, test.OldExpectedURL)
-			}
-			t.Logf(`%v %v(%v) = %q`, greenTick(), test.FunctionType, quotedList(oldRootURL, argSet), result)
 		}
 	}
 }
 
-// quotedList returns a quoted list of the arguments passed in
+// quotedList returns a backtick-quoted list of the arguments passed in
 func quotedList(url string, args []string) string {
 	all := append([]string{url}, args...)
-	return `'` + strings.Join(all, `', '`) + `'`
+	return "`" + strings.Join(all, "`, `") + "`"
 }
 
 // greenTick returns an ANSI string including escape codes to render a light
 // green tick (✓) in a color console
 func greenTick() string {
 	return string([]byte{0x1b, 0x5b, 0x33, 0x32, 0x6d, 0xe2, 0x9c, 0x93, 0x1b, 0x5b, 0x30, 0x6d})
 }
+
+func redCross() string {
+	return "❌"
+}