Merge pull request #162 from vania-pooh/master

Added ability to correctly process complex version ranges
aerokube · May 31, 2018 · 3bda2c2 · 3bda2c2
2 parents 5d882f4 + fa8ea9d
commit 3bda2c2
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 37 deletions.
diff --git a/docs/selenoid-commands.adoc b/docs/selenoid-commands.adoc
@@ -112,7 +112,7 @@ This command does nothing when already downloaded. Use `--force` flag to downloa
 +
 [source,bash]
 ----
-./cm selenoid configure [--browsers firefox:>45.0,opera,android] [--last-versions 2] [--tmpfs 128]
+./cm selenoid configure [--browsers firefox:>45.0;opera:53.0;android] [--last-versions 2] [--tmpfs 128]
 ----
 +    
 Use `--browsers` to limit browsers to be configured, `--tmpfs` - to add https://en.wikipedia.org/wiki/Tmpfs[Tmpfs] support, `--last-versions` - to limit how many last browser versions to download. If you wish to download all available versions - specify `--last-versions 0`.

diff --git a/selenoid/docker.go b/selenoid/docker.go
@@ -26,7 +26,7 @@ import (
 	"github.com/docker/go-connections/nat"
 	ver "github.com/hashicorp/go-version"
 	"github.com/heroku/docker-registry-client/registry"
-	colorable "github.com/mattn/go-colorable"
+	"github.com/mattn/go-colorable"
 
 	"net/http"
 	"path/filepath"
@@ -43,6 +43,8 @@ import (
 )
 
 const (
+	semicolon               = ";"
+	colon                   = ":"
 	Latest                  = "latest"
 	firefox                 = "firefox"
 	android                 = "android"
@@ -321,7 +323,7 @@ func (c *DockerConfigurator) Configure() (*SelenoidConfig, error) {
 }
 
 func (c *DockerConfigurator) createConfig() SelenoidConfig {
-	requestedBrowsers := c.parseRequestedBrowsers(c.Browsers)
+	requestedBrowsers := parseRequestedBrowsers(&c.Logger, c.Browsers)
 	browsersToIterate := c.getBrowsersToIterate(requestedBrowsers)
 	browsers := make(map[string]config.Versions)
 	for browserName, image := range browsersToIterate {
@@ -348,46 +350,51 @@ func (c *DockerConfigurator) createConfig() SelenoidConfig {
 	return browsers
 }
 
-func (c *DockerConfigurator) parseRequestedBrowsers(requestedBrowsers string) map[string]*ver.Constraints {
+func parseRequestedBrowsers(logger *Logger, requestedBrowsers string) map[string]*ver.Constraints {
 	ret := make(map[string]*ver.Constraints)
-	for _, section := range strings.Split(requestedBrowsers, comma) {
-		pieces := strings.Split(section, colon)
-		if len(pieces) == 2 {
-			browserName := pieces[0]
-			versionConstraintString := pieces[1]
-			versionConstraint, err := ver.NewConstraint(versionConstraintString)
-			if err != nil {
-				c.Errorf("Invalid version constraint %s: %v - ignoring browser %s...", versionConstraintString, err, browserName)
-				continue
+	if requestedBrowsers != "" {
+		for _, section := range strings.Split(requestedBrowsers, semicolon) {
+			pieces := strings.Split(section, colon)
+			if len(pieces) == 2 {
+				browserName := pieces[0]
+				versionConstraintString := pieces[1]
+				versionConstraint, err := ver.NewConstraint(versionConstraintString)
+				if err != nil {
+					logger.Errorf(`Invalid version constraint %s: %v - ignoring browser "%s"...`, versionConstraintString, err, browserName)
+					continue
+				}
+				ret[browserName] = &versionConstraint
+			} else if len(pieces) == 1 {
+				browserName := pieces[0]
+				ret[browserName] = nil
 			}
-			ret[browserName] = &versionConstraint
-		} else if len(pieces) == 1 {
-			browserName := pieces[0]
-			ret[browserName] = nil
 		}
 	}
 	return ret
 }
 
 func (c *DockerConfigurator) getBrowsersToIterate(requestedBrowsers map[string]*ver.Constraints) map[string]string {
-	ret := make(map[string]string)
 	defaultBrowsers := map[string]string{
 		"firefox": "selenoid/firefox",
 		"chrome":  "selenoid/chrome",
 		"opera":   "selenoid/opera",
 	}
-	for browserName := range requestedBrowsers {
-		if image, ok := defaultBrowsers[browserName]; ok {
-			ret[browserName] = image
-			continue
+	if len(requestedBrowsers) > 0 {
+		ret := make(map[string]string)
+		for browserName := range requestedBrowsers {
+			if image, ok := defaultBrowsers[browserName]; ok {
+				ret[browserName] = image
+				continue
+			}
+			c.Errorf("Unsupported browser: %s", browserName)
 		}
-		c.Errorf("Unsupported browser: %s", browserName)
-	}
 
-	if _, ok := requestedBrowsers[android]; ok {
-		ret["android"] = "selenoid/android"
+		if _, ok := requestedBrowsers[android]; ok {
+			ret["android"] = "selenoid/android"
+		}
+		return ret
 	}
-	return ret
+	return defaultBrowsers
 }
 
 func (c *DockerConfigurator) fetchImageTags(image string) []string {

diff --git a/selenoid/docker_test.go b/selenoid/docker_test.go
@@ -261,7 +261,7 @@ func testConfigure(t *testing.T, download bool) {
 			Quiet:        false,
 			LastVersions: 2,
 			Tmpfs:        512,
-			Browsers:     "firefox:>45.0,opera",
+			Browsers:     "firefox:>45.0;opera",
 			Args:         "-limit 42",
 			VNC:          true,
 			Env:          testEnv,

diff --git a/selenoid/drivers.go b/selenoid/drivers.go
@@ -30,8 +30,6 @@ import (
 const (
 	zipMagicHeader  = "504b"
 	gzipMagicHeader = "1f8b"
-	comma           = ","
-	colon           = ":"
 	owner           = "aerokube"
 	selenoidRepo    = "selenoid"
 	selenoidUIRepo  = "selenoid-ui"
@@ -507,18 +505,18 @@ func outputFile(outputPath string, mode os.FileMode, r io.Reader) error {
 }
 
 func (d *DriversConfigurator) downloadDrivers(browsers *Browsers, configDir string) []downloadedDriver {
-	ret := []downloadedDriver{}
+	var ret []downloadedDriver
 	browsersToIterate := *browsers
 	if d.Browsers != "" {
-		requestedBrowsers := strings.Split(d.Browsers, comma)
+		requestedBrowsers := parseRequestedBrowsers(&d.Logger, d.Browsers)
 		if len(requestedBrowsers) > 0 {
 			browsersToIterate = make(Browsers)
-			for _, rb := range requestedBrowsers {
-				if browser, ok := (*browsers)[rb]; ok {
-					browsersToIterate[rb] = browser
+			for browserName := range requestedBrowsers {
+				if browser, ok := (*browsers)[browserName]; ok {
+					browsersToIterate[browserName] = browser
 					continue
 				}
-				d.Errorf("Unsupported browser: %s", rb)
+				d.Errorf("Unsupported browser: %s", browserName)
 			}
 		}
 	}

diff --git a/selenoid/drivers_test.go b/selenoid/drivers_test.go
@@ -169,7 +169,7 @@ func TestConfigureDrivers(t *testing.T) {
 		browsersJsonUrl := mockServerUrl(mockDriverServer, "/browsers.json")
 		lcConfig := LifecycleConfig{
 			ConfigDir:       dir,
-			Browsers:        "first,second,safari,fourth",
+			Browsers:        "first;second;safari;fourth",
 			BrowsersJsonUrl: browsersJsonUrl,
 			Download:        true,
 			Quiet:           false,