Repository: ddev/ddev
Pull request #6693 (2024-11-06 00:47:00)

test: run full tests that are sometimes short-circuited
## The Issue

In studying the test run and coverage for 
* https://github.com/ddev/ddev/pull/6691

I was a little suspicious that not all PHP versions had been tested. 

Sure enough, we have a number of costly tests that only run the full set of possibilities when GOTEST_SHORT is empty.

So the only test that would have tried all my PHP versions was TestPHPConfig, and it was short-circuited in all our tests.

We only run the most costly tests without short circuits in GitHub tests.yml `pull-push-test-platforms`, so the tests that are like this have to be explicitly mentioned there. 

Then I discovered that the logic in some tests (most importantly TestPHPConfig) was incorrect, so when GOTEST_SHORT="" it wasn't running *any* of the PHP versions.

## How This PR Solves The Issue

* Look for tests of this behavior
* Add them to the list
* Reorganize the list so it's more readable.
* Fix tests that weren't running the right thing on GOTEST_SHORT=



github-actions[bot]: <!-- bot: nightly-link -->
Download the artifacts for this pull request:

* [all-ddev-executables.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149835608.zip)
* [ddev-macos-amd64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149835682.zip)
* [ddev-macos-arm64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149835758.zip)
* [ddev-linux-arm64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149835816.zip)
* [ddev-linux-amd64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149835881.zip)
* [ddev-windows-amd64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149835953.zip)
* [ddev-windows-amd64-installer.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149836005.zip)
* [ddev-windows-arm64.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149836081.zip)
* [ddev-windows-arm64-installer.zip](https://nightly.link/ddev/ddev/actions/artifacts/2149836139.zip)

See [Testing a PR](https://ddev.readthedocs.io/en/latest/developers/building-contributing/#testing-a-pr).

[![Open in Gitpod](https://gitpod.io/button/open-in-gitpod.svg)](https://gitpod.io/#https://github.com/ddev/ddev/pull/6693)

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://github.com/codespaces/new?ref=20241105_run_more_tests&repo=80669528)

Patch:
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 42797481d..35e26a966 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -419,3 +419,22 @@ func GetTimezone(path string) (string, error) {
 	}
 	return timezone, nil
 }
+
+// SubtractSlices removes elements of slice b from slice a.
+func SubtractSlices(a, b []string) []string {
+	// Create a map to keep track of elements in slice b for quick lookup.
+	bMap := make(map[string]bool)
+	for _, elem := range b {
+		bMap[elem] = true
+	}
+
+	// Collect elements from a that are not in b.
+	var result []string
+	for _, elem := range a {
+		if _, found := bMap[elem]; !found {
+			result = append(result, elem)
+		}
+	}
+
+	return result
+}


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index fd345d6fd..f17d00264 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -124,8 +124,26 @@ jobs:
       - name: Override environment variables for push-pull-test-platforms
         run: |
           echo "MAKE_TARGET=test" >> $GITHUB_ENV
-          echo "TESTARGS=-failfast -run '(TestDdevFullSite.*|TestDdevImportFiles|TestDdevAllDatabases|TestComposerCreateCmd|Test.*(Push|Pull)|TestAutocomplet)'" >> $GITHUB_ENV
           echo "GOTEST_SHORT=" >> $GITHUB_ENV
+          # In push-pull-test-platforms we want to run all tests
+          # that limit their scope when running with any value of GOTEST_SHORT
+          TP=(
+            "TestAppdirAlreadyInUse"
+            "TestAutocomplet"
+            "TestAvailableUpdates"
+            "TestComposerCreateCmd"
+            "TestConfigDatabaseVersion"
+            "TestDdevAllDatabases"
+            "TestDdevFullSite.*"
+            "TestDdevImportFiles"
+            "TestDdevXdebugEnabled"
+            "TestDdevXhprofEnabled"
+            "TestPHPConfig"
+            "Test.*(Push|Pull)"
+          )
+          TESTARGS="-failfast -run '($(IFS="|"; echo "${TP[*]}"))'"
+          echo "pull-push-test-platforms TESTARGS=$TESTARGS"
+          echo "TESTARGS=${TESTARGS}" >> $GITHUB_ENV
         if: ${{ matrix.pull-push-test-platforms }}
 
       - name: Load 1password secret(s) for push-pull-test-platforms
diff --git a/pkg/ddevapp/config_test.go b/pkg/ddevapp/config_test.go
index d64edd071..41afdde3f 100644
--- a/pkg/ddevapp/config_test.go
+++ b/pkg/ddevapp/config_test.go
@@ -846,12 +846,11 @@ func TestPHPConfig(t *testing.T) {
 	})
 
 	// Most of the time there's no reason to do all versions of PHP
-	phpKeys := []string{}
+	// so we can subtract those if GOTEST_SHORT==""
+	phpKeys := nodeps.GetValidPHPVersions()
 	exclusions := []string{nodeps.PHP56, nodeps.PHP70, nodeps.PHP71, nodeps.PHP72, nodeps.PHP73, nodeps.PHP74, nodeps.PHP80, nodeps.PHP81}
-	for k := range nodeps.ValidPHPVersions {
-		if os.Getenv("GOTEST_SHORT") != "" && !nodeps.ArrayContainsString(exclusions, k) {
-			phpKeys = append(phpKeys, k)
-		}
+	if os.Getenv("GOTEST_SHORT") != "" {
+		phpKeys = util.SubtractSlices(phpKeys, exclusions)
 	}
 	sort.Strings(phpKeys)
 
diff --git a/pkg/ddevapp/ddevapp_test.go b/pkg/ddevapp/ddevapp_test.go
index 752502639..8f32c9695 100644
--- a/pkg/ddevapp/ddevapp_test.go
+++ b/pkg/ddevapp/ddevapp_test.go
@@ -952,20 +952,15 @@ func TestDdevXdebugEnabled(t *testing.T) {
 
 	testcommon.ClearDockerEnv()
 
-	// Most of the time there's no reason to do all versions of PHP
-	phpKeys := []string{}
-	exclusions := []string{"5.6", "7.0", "7.1", "7.2", "7.3", "7.4", "8.0"}
-	for k := range nodeps.ValidPHPVersions {
-		// TODO: Remove when xdebug available for php8.4
-		if k == nodeps.PHP84 {
-			continue
-		}
-		if os.Getenv("GOTEST_SHORT") != "" && !nodeps.ArrayContainsString(exclusions, k) {
-			phpKeys = append(phpKeys, k)
-		}
-	}
+	// Default to testing all versions if GOTEST_SHORT is not set
+	phpKeys := nodeps.GetValidPHPVersions()
+
+	// TODO: Remove the PHP 8.4 exclusion when php8.4-xhprof is available
+	exclusions := []string{nodeps.PHP84}
+	phpKeys = util.SubtractSlices(phpKeys, exclusions)
 	sort.Strings(phpKeys)
-	// Most of the time we can just test with the default PHP version
+
+	// Test only the default version if GOTEST_SHORT is set
 	if os.Getenv("GOTEST_SHORT") != "" {
 		phpKeys = []string{nodeps.PHPDefault}
 	}
@@ -1121,20 +1116,13 @@ func TestDdevXhprofEnabled(t *testing.T) {
 
 	// Does not work with php5.6 anyway (SEGV), for resource conservation
 	// skip older unsupported versions
-	phpKeys := []string{}
-	exclusions := []string{"5.6", "7.0", "7.1", "7.2", "7.3", "7.4", "8.0"}
-	for k := range nodeps.ValidPHPVersions {
-		// TODO: Remove when xhprof available for php8.4
-		if k == nodeps.PHP84 {
-			continue
-		}
-		if !nodeps.ArrayContainsString(exclusions, k) {
-			phpKeys = append(phpKeys, k)
-		}
-	}
+	phpKeys := nodeps.GetValidPHPVersions()
+	// TODO: Remove the PHP 8.4 exclusion when php8.4-xhprof is available
+	exclusions := []string{nodeps.PHP56, nodeps.PHP84}
+	phpKeys = util.SubtractSlices(phpKeys, exclusions)
 	sort.Strings(phpKeys)
 
-	// Most of the time we can just test with the default PHP version
+	// If GOTESt_SHORT is set, we'll just use the default version instead
 	if os.Getenv("GOTEST_SHORT") != "" {
 		phpKeys = []string{nodeps.PHPDefault}
 	}
@@ -1658,10 +1646,12 @@ func TestDdevAllDatabases(t *testing.T) {
 
 	dbVersions := nodeps.GetValidDatabaseVersions()
 	// Bug: PostgreSQL 9 doesn't work with snapshot restore, see https://github.com/ddev/ddev/issues/3583
-	dbVersions = nodeps.RemoveItemFromSlice(dbVersions, "postgres:9")
+	exclusions := []string{"postgres:9"}
+	dbVersions = util.SubtractSlices(dbVersions, exclusions)
+
 	//Use a smaller list if GOTEST_SHORT
 	if os.Getenv("GOTEST_SHORT") != "" {
-		dbVersions = []string{"postgres:14", "mariadb:10.11", "mariadb:10.6", "mariadb:10.4", "mysql:8.0", "mysql:5.7"}
+		dbVersions = []string{"postgres:17", "mariadb:10.11", "mariadb:10.6", "mysql:8.0", "mysql:5.7"}
 		t.Logf("Using limited set of database servers because GOTEST_SHORT is set (%v)", dbVersions)
 	}
 
diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go
index 8139b6258..a36a93023 100644
--- a/pkg/util/utils_test.go
+++ b/pkg/util/utils_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"sort"
 	"strings"
 	"testing"
 
@@ -230,3 +231,59 @@ func TestGetTimezone(t *testing.T) {
 		})
 	}
 }
+
+// compareSlices checks if two slices are equal after sorting them.
+func compareSlices(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	// Sort both slices before comparing
+	sort.Strings(a)
+	sort.Strings(b)
+
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+// TestSubtractSlices does simple test of SubtractSlices
+func TestSubtractSlices(t *testing.T) {
+	tests := []struct {
+		name     string
+		a        []string
+		b        []string
+		expected []string
+	}{
+		{
+			name:     "No overlap",
+			a:        []string{"apple", "banana", "cherry"},
+			b:        []string{"date", "fig"},
+			expected: []string{"apple", "banana", "cherry"},
+		},
+		{
+			name:     "Partial overlap",
+			a:        []string{"apple", "banana", "cherry"},
+			b:        []string{"banana"},
+			expected: []string{"apple", "cherry"},
+		},
+		{
+			name:     "Complete overlap",
+			a:        []string{"apple", "banana"},
+			b:        []string{"apple", "banana"},
+			expected: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := util.SubtractSlices(tt.a, tt.b)
+			if !compareSlices(result, tt.expected) {
+				t.Errorf("SubtractSlices(%v, %v) = %v; want %v", tt.a, tt.b, result, tt.expected)
+			}
+		})
+	}
+}