DEV: Improve assertions of cli_build_test.go (#844)

This commit improves the assertions by testing against the entire
command string and env so that we can be sure of the full command we are
running.

Author: tgxworld

launcher_go/v2/cli_build.go

@@ -3,12 +3,14 @@ package main
 import (
 	"context"
 	"errors"
+	"flag"
+	"os"
+	"strings"
+
 	"github.com/discourse/discourse_docker/launcher_go/v2/config"
 	"github.com/discourse/discourse_docker/launcher_go/v2/docker"
 	"github.com/discourse/discourse_docker/launcher_go/v2/utils"
 	"github.com/google/uuid"
-	"os"
-	"strings"
 )
 
 /*
@@ -66,7 +68,16 @@ func (r *DockerConfigureCmd) Run(cli *Cli, ctx *context.Context) error {
 		return errors.New("YAML syntax error. Please check your containers/*.yml config files.")
 	}
 
-	containerId := "discourse-build-" + uuid.NewString()
+	var uuidString string
+
+	if flag.Lookup("test.v") == nil {
+		uuidString = uuid.NewString()
+	} else {
+		uuidString = "test"
+	}
+
+	containerId := "discourse-build-" + uuidString
+
 	pups := docker.DockerPupsRunner{
 		Config:         config,
 		PupsArgs:       "--tags=db,precompile",
@@ -75,6 +86,7 @@ func (r *DockerConfigureCmd) Run(cli *Cli, ctx *context.Context) error {
 		Ctx:            ctx,
 		ContainerId:    containerId,
 	}
+
 	return pups.Run()
 }
 

launcher_go/v2/cli_build_test.go

@@ -59,33 +59,89 @@ var _ = Describe("Build", func() {
 		}
 
 		var checkConfigureCmd = func(cmd exec.Cmd) {
-			Expect(cmd.String()).To(ContainSubstring("docker run"))
-			Expect(cmd.String()).To(ContainSubstring("--env DISCOURSE_DEVELOPER_EMAILS"))
-			Expect(cmd.String()).To(ContainSubstring("--env SKIP_EMBER_CLI_COMPILE=1"))
-			// we commit, we need the container to stick around after it is stopped.
-			Expect(cmd.String()).ToNot(ContainSubstring("--rm"))
-
-			// we don't expose ports on configure command
-			Expect(cmd.String()).ToNot(ContainSubstring("-p 80"))
-			Expect(cmd.Env).To(ContainElement("DISCOURSE_DB_PASSWORD=SOME_SECRET"))
+			Expect(cmd.String()).To(Equal(
+				"/usr/local/bin/docker run " +
+					"--env DISCOURSE_DB_HOST " +
+					"--env DISCOURSE_DB_PASSWORD " +
+					"--env DISCOURSE_DB_PORT " +
+					"--env DISCOURSE_DB_SOCKET " +
+					"--env DISCOURSE_DEVELOPER_EMAILS " +
+					"--env DISCOURSE_HOSTNAME " +
+					"--env DISCOURSE_REDIS_HOST " +
+					"--env DISCOURSE_SMTP_ADDRESS " +
+					"--env DISCOURSE_SMTP_PASSWORD " +
+					"--env DISCOURSE_SMTP_USER_NAME " +
+					"--env LANG " +
+					"--env LANGUAGE " +
+					"--env LC_ALL " +
+					"--env MULTI " +
+					"--env RAILS_ENV " +
+					"--env REPLACED " +
+					"--env RUBY_GC_HEAP_GROWTH_MAX_SLOTS " +
+					"--env RUBY_GC_HEAP_INIT_SLOTS " +
+					"--env RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR " +
+					"--env UNICORN_SIDEKIQS " +
+					"--env UNICORN_WORKERS " +
+					"--env SKIP_EMBER_CLI_COMPILE=1 " +
+					"--volume /var/discourse/shared/web-only:/shared " +
+					"--volume /var/discourse/shared/web-only/log/var-log:/var/log " +
+					"--link data:data " +
+					"--shm-size=512m " +
+					"--restart=no " +
+					"--interactive " +
+					"--expose 100 " +
+					"--name discourse-build-test " +
+					"local_discourse/test /bin/bash -c /usr/local/bin/pups --stdin --tags=db,precompile",
+			))
+
+			Expect(cmd.Env).To(Equal([]string{
+				"DISCOURSE_DB_HOST=data",
+				"DISCOURSE_DB_PASSWORD=SOME_SECRET",
+				"DISCOURSE_DB_PORT=",
+				"DISCOURSE_DB_SOCKET=",
+				"[email protected],[email protected]",
+				"DISCOURSE_HOSTNAME=discourse.example.com",
+				"DISCOURSE_REDIS_HOST=data",
+				"DISCOURSE_SMTP_ADDRESS=smtp.example.com",
+				"DISCOURSE_SMTP_PASSWORD=pa$$word",
+				"[email protected]",
+				"LANG=en_US.UTF-8",
+				"LANGUAGE=en_US.UTF-8",
+				"LC_ALL=en_US.UTF-8",
+				"MULTI=test\nmultiline with some spaces\nvar\n",
+				"RAILS_ENV=production",
+				"REPLACED=test/test/test",
+				"RUBY_GC_HEAP_GROWTH_MAX_SLOTS=40000",
+				"RUBY_GC_HEAP_INIT_SLOTS=400000",
+				"RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR=1.5",
+				"UNICORN_SIDEKIQS=1",
+				"UNICORN_WORKERS=3",
+			}))
+
 			buf := new(strings.Builder)
 			io.Copy(buf, cmd.Stdin)
 			// docker run's stdin is a pups config
+
+			// web.template.yml is merged with the test config
 			Expect(buf.String()).To(ContainSubstring("path: /etc/service/nginx/run"))
+			Expect(buf.String()).To(ContainSubstring(`exec: echo "custom test command"`))
 		}
 
 		// commit on configure
 		var checkConfigureCommit = func(cmd exec.Cmd) {
-			Expect(cmd.String()).To(ContainSubstring("docker commit"))
-			Expect(cmd.String()).To(ContainSubstring("--change CMD [\"/sbin/boot\"]"))
-			Expect(cmd.String()).To(ContainSubstring("discourse-build"))
-			Expect(cmd.String()).To(ContainSubstring("local_discourse/test"))
-			Expect(cmd.Env).ToNot(ContainElement("DISCOURSE_DB_PASSWORD=SOME_SECRET"))
+			Expect(cmd.String()).To(MatchRegexp(
+				"/usr/local/bin/docker commit " +
+					`--change LABEL org\.opencontainers\.image\.created="[\d\-T:+]+" ` +
+					`--change CMD \["/sbin/boot"\] ` +
+					"discourse-build-test local_discourse/test",
+			))
+
+			Expect(cmd.Env).To(BeNil())
 		}
 
 		// configure also cleans up
 		var checkConfigureClean = func(cmd exec.Cmd) {
-			Expect(cmd.String()).To(ContainSubstring("docker rm -f discourse-build-"))
+			Expect(cmd.String()).To(Equal("/usr/local/bin/docker rm --force discourse-build-test"))
 		}
 
 		It("Should run docker build with correct arguments", func() {
@@ -99,6 +155,7 @@ var _ = Describe("Build", func() {
 			runner := ddocker.DockerConfigureCmd{Config: "test"}
 			runner.Run(cli, &ctx)
 			Expect(len(RanCmds)).To(Equal(3))
+
 			checkConfigureCmd(RanCmds[0])
 			checkConfigureCommit(RanCmds[1])
 			checkConfigureClean(RanCmds[2])

launcher_go/v2/config/config.go

@@ -1,16 +1,17 @@
 package config
 
 import (
-	"dario.cat/mergo"
 	"errors"
 	"fmt"
-	"github.com/discourse/discourse_docker/launcher_go/v2/utils"
 	"os"
 	"regexp"
 	"runtime"
 	"slices"
 	"strings"
 
+	"dario.cat/mergo"
+	"github.com/discourse/discourse_docker/launcher_go/v2/utils"
+
 	"gopkg.in/yaml.v3"
 )
 
@@ -77,7 +78,9 @@ func LoadConfig(dir string, configName string, includeTemplates bool, templatesD
 		Boot_Command: defaultBootCommand,
 		Base_Image:   DefaultBaseImage(),
 	}
+
 	matched, _ := regexp.MatchString("[[:upper:]/ !@#$%^&*()+~`=]", configName)
+
 	if matched {
 		msg := "ERROR: Config name '" + configName + "' must not contain upper case characters, spaces or special characters. Correct config name and rerun."
 		fmt.Println(msg)
@@ -86,12 +89,14 @@ func LoadConfig(dir string, configName string, includeTemplates bool, templatesD
 
 	config_filename := string(strings.TrimRight(dir, "/") + "/" + config.Name + ".yml")
 	content, err := os.ReadFile(config_filename)
+
 	if err != nil {
 		if os.IsNotExist(err) {
 			fmt.Println("config file does not exist: " + config_filename)
 		}
 		return nil, err
 	}
+
 	baseConfig := &Config{}
 
 	if err := yaml.Unmarshal(content, baseConfig); err != nil {
@@ -105,10 +110,13 @@ func LoadConfig(dir string, configName string, includeTemplates bool, templatesD
 			}
 		}
 	}
+
 	if err := mergo.Merge(config, baseConfig, mergo.WithOverride); err != nil {
 		return nil, err
 	}
+
 	config.rawYaml = append(config.rawYaml, string(content[:]))
+
 	if err != nil {
 		return nil, err
 	}

launcher_go/v2/docker/commands.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"runtime"
+	"sort"
 	"strings"
 	"syscall"
 	"time"
@@ -94,19 +95,28 @@ func (r *DockerRunner) Run() error {
 	}
 
 	cmd.Env = r.Config.EnvArray(true)
+	envKeys := make([]string, 0, len(r.Config.Env))
+
+	for envKey := range r.Config.Env {
+		envKeys = append(envKeys, envKey)
+	}
+
+	sort.Strings(envKeys)
 
 	if r.DryRun {
 		// multi-line env doesn't work super great from CLI, but we can print out the rest.
-		for k, v := range r.Config.Env {
-			if !strings.Contains(v, "\n") {
+		for _, envKey := range envKeys {
+			value := r.Config.Env[envKey]
+
+			if !strings.Contains(value, "\n") {
 				cmd.Args = append(cmd.Args, "--env")
-				cmd.Args = append(cmd.Args, k+"="+shellwords.Escape(v))
+				cmd.Args = append(cmd.Args, envKey+"="+shellwords.Escape(value))
 			}
 		}
 	} else {
-		for k, _ := range r.Config.Env {
+		for _, envKey := range envKeys {
 			cmd.Args = append(cmd.Args, "--env")
-			cmd.Args = append(cmd.Args, k)
+			cmd.Args = append(cmd.Args, envKey)
 		}
 	}
 
@@ -156,10 +166,10 @@ func (r *DockerRunner) Run() error {
 	}
 
 	if r.Detatch {
-		cmd.Args = append(cmd.Args, "-d")
+		cmd.Args = append(cmd.Args, "--detach")
 	}
 
-	cmd.Args = append(cmd.Args, "-i")
+	cmd.Args = append(cmd.Args, "--interactive")
 
 	// Docker args override settings above
 	for _, f := range r.Config.DockerArgs() {
@@ -170,8 +180,11 @@ func (r *DockerRunner) Run() error {
 		cmd.Args = append(cmd.Args, f)
 	}
 
-	cmd.Args = append(cmd.Args, "-h")
-	cmd.Args = append(cmd.Args, r.Hostname)
+	if r.Hostname != "" {
+		cmd.Args = append(cmd.Args, "--hostname")
+		cmd.Args = append(cmd.Args, r.Hostname)
+	}
+
 	cmd.Args = append(cmd.Args, "--name")
 	cmd.Args = append(cmd.Args, r.ContainerId)
 
@@ -216,21 +229,26 @@ func (r *DockerPupsRunner) Run() error {
 	rm := false
 	// remove : in case docker tag is blank, and use default latest tag
 	r.SavedImageName = strings.TrimRight(r.SavedImageName, ":")
+
 	if r.SavedImageName == "" {
 		rm = true
 	}
+
 	defer func(rm bool) {
 		if !rm {
 			time.Sleep(utils.CommitWait)
 			runCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-			cmd := exec.CommandContext(runCtx, utils.DockerPath, "rm", "-f", r.ContainerId)
+			cmd := exec.CommandContext(runCtx, utils.DockerPath, "rm", "--force", r.ContainerId)
 			utils.CmdRunner(cmd).Run()
 			cancel()
 		}
 	}(rm)
-	commands := []string{"/bin/bash",
+
+	commands := []string{
+		"/bin/bash",
 		"-c",
-		"/usr/local/bin/pups --stdin " + r.PupsArgs}
+		"/usr/local/bin/pups --stdin " + r.PupsArgs,
+	}
 
 	runner := DockerRunner{Config: r.Config,
 		Ctx:         r.Ctx,
@@ -248,6 +266,7 @@ func (r *DockerPupsRunner) Run() error {
 
 	if len(r.SavedImageName) > 0 {
 		time.Sleep(utils.CommitWait)
+
 		cmd := exec.Command("docker",
 			"commit",
 			"--change",
@@ -257,13 +276,16 @@ func (r *DockerPupsRunner) Run() error {
 			r.ContainerId,
 			r.SavedImageName,
 		)
+
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
 
 		fmt.Fprintln(utils.Out, cmd)
+
 		if err := utils.CmdRunner(cmd).Run(); err != nil {
 			return err
 		}
 	}
+
 	return nil
 }

launcher_go/v2/test/containers/test.yml

@@ -81,7 +81,7 @@ env:
   ## The http or https CDN address for this Discourse instance (configured to pull)
   ## see https://meta.discourse.org/t/14857 for details
   #DISCOURSE_CDN_URL: https://discourse-cdn.example.com
-  
+
   ## The maxmind geolocation IP address key for IP address lookup
   ## see https://meta.discourse.org/t/-/137387/23 for details
   #DISCOURSE_MAXMIND_LICENSE_KEY: 1234567890123456
@@ -105,6 +105,7 @@ hooks:
 
 ## Remember, this is YAML syntax - you can only have one block with a name
 run:
+  - exec: echo "custom test command"
   - exec: echo "Beginning of custom commands"
 
   ## If you want to configure password login for root, uncomment and change: