Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Parallelize change detector #1871

Merged
merged 24 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
605f428
move change detector logic to tests/change_detector so it can run tes…
nasdf Sep 12, 2023
de1a1eb
set rootDatabaseDir from env variable
nasdf Sep 12, 2023
5b389cb
set databaseDir instead of rootDatabaseDir
nasdf Sep 12, 2023
f6bb25b
cleanup database file path unused variables
nasdf Sep 12, 2023
dc5aaf8
set count flag in go test command
nasdf Sep 12, 2023
6ab7371
always set detect changes env
nasdf Sep 12, 2023
8195242
fix bug in badger file path
nasdf Sep 12, 2023
01afa55
restore preflight checks and db path logic
nasdf Sep 12, 2023
4b9c36e
enable parallel change detector
nasdf Sep 12, 2023
9ec616a
add more documentation
nasdf Sep 12, 2023
3b84d9f
update Makefile test targets
nasdf Sep 12, 2023
536f6fe
move all change detector logic to new package. clean up env names and…
nasdf Sep 12, 2023
b312303
replace panic with test.Fail in checkIfDatabaseFormatChangesAreDocume…
nasdf Sep 12, 2023
6039650
update test:changes Makefile target test flags
nasdf Sep 12, 2023
f6af08b
add change_detector build flag. remove shuffle from change_detector M…
nasdf Sep 18, 2023
eb80d80
change detector target defaults to local repo
nasdf Sep 18, 2023
97396de
document change detector changes. set default branch and repo to upst…
nasdf Sep 18, 2023
08a1186
Merge branch 'develop' into nasdf/test/parallel-change-detector
nasdf Sep 18, 2023
fc6cbcd
fix linter errors
nasdf Sep 18, 2023
cfc1182
fix build errors
nasdf Sep 18, 2023
de3e575
Merge branch 'develop' into nasdf/test/parallel-change-detector
nasdf Sep 18, 2023
b01d301
add config logging
nasdf Sep 19, 2023
3626037
move test logging to executeTestCase
nasdf Sep 19, 2023
cf6267f
Merge branch 'develop' into nasdf/test/parallel-change-detector
nasdf Sep 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ TEST_FLAGS=-race -shuffle=on -timeout 300s
PLAYGROUND_DIRECTORY=playground
LENS_TEST_DIRECTORY=tests/integration/schema/migrations
CLI_TEST_DIRECTORY=tests/integration/cli
DEFAULT_TEST_DIRECTORIES=$$(go list ./... | grep -v -e $(LENS_TEST_DIRECTORY) -e $(CLI_TEST_DIRECTORY))
CHANGE_DETECTOR_TEST_DIRECTORY=tests/change_detector
DEFAULT_TEST_DIRECTORIES=$$(go list ./... | grep -v -e $(LENS_TEST_DIRECTORY) -e $(CLI_TEST_DIRECTORY) -e $(CHANGE_DETECTOR_TEST_DIRECTORY))

default:
@go run $(BUILD_FLAGS) cmd/defradb/main.go
Expand Down Expand Up @@ -294,8 +295,7 @@ test\:coverage-html:

.PHONY: test\:changes
test\:changes:
@$(MAKE) deps:lens
env DEFRA_DETECT_DATABASE_CHANGES=true DEFRA_CLIENT_GO=true gotestsum -- ./... -shuffle=on -p 1
gotestsum --format testname -- ./$(CHANGE_DETECTOR_TEST_DIRECTORY)/... -shuffle=on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: It does look like -shuffle=on adds no value here? Or is it passed down into the child tests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed shuffle flag.


.PHONY: validate\:codecov
validate\:codecov:
Expand Down
15 changes: 15 additions & 0 deletions tests/change_detector/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Change Detector

The change detector is used to detect data format changes between versions of DefraDB.

## How it works

The tests run using a `source` and `target` branch of DefraDB. Each branch is cloned into a temporary directory and dependencies are installed.

The test runner executes all of the common test packages available in the `source` and `target` tests directory.

For each test package execution the following steps occur:

- Create a temporary data directory. This is used to share data between `source` and `target`.
- Run the `source` version in setup only mode. This creates test fixtures in the shared data directory.
- Run the `target` version in change detector mode. This skips the setup and executes the tests using the shared data directory.
121 changes: 121 additions & 0 deletions tests/change_detector/change_detector_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2023 Democratized Data Foundation
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package change_detector
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion(non-blocking, minor): Whilst I like the approach, I do dislike that this test will be executed by default with go test ./..., I think it might be nicer if we exclude this test by default, perhaps using a build flag. This would however complicate the github action matrix stuff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added build tag here and updated Makefile to include --tags change_detector.


import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/require"
)

func TestChanges(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: I really like this approach, it is simpler and neater than some kind of file lock IMO.

sourceRepoDir := t.TempDir()
execClone(t, sourceRepoDir, repository, sourceBranch)

if checkIfDatabaseFormatChangesAreDocumented(t, sourceRepoDir) {
t.Skip("skipping test with documented database format changes")
}

targetRepoDir := t.TempDir()
execClone(t, targetRepoDir, repository, targetBranch)

execMakeDeps(t, sourceRepoDir)
execMakeDeps(t, targetRepoDir)

targetRepoTestDir := filepath.Join(targetRepoDir, "tests", "integration")
targetRepoPkgList := execList(t, targetRepoTestDir)

sourceRepoTestDir := filepath.Join(sourceRepoDir, "tests", "integration")
sourceRepoPkgList := execList(t, sourceRepoTestDir)

sourceRepoPkgMap := make(map[string]bool)
for _, pkg := range sourceRepoPkgList {
sourceRepoPkgMap[pkg] = true
}

for _, pkg := range targetRepoPkgList {
pkgName := strings.TrimPrefix(pkg, "github.com/sourcenetwork/defradb/")
t.Run(pkgName, func(t *testing.T) {
if pkg == "" || !sourceRepoPkgMap[pkg] {
t.Skip("skipping unknown or new test package")
}

t.Parallel()
dataDir := t.TempDir()

sourceTestPkg := filepath.Join(sourceRepoDir, pkgName)
execTest(t, sourceTestPkg, dataDir, true)

targetTestPkg := filepath.Join(targetRepoDir, pkgName)
execTest(t, targetTestPkg, dataDir, false)
})
}
}

// execList returns a list of all packages in the given directory.
func execList(t *testing.T, dir string) []string {
cmd := exec.Command("go", "list", "./...")
cmd.Dir = dir

out, err := cmd.Output()
require.NoError(t, err, string(out))

return strings.Split(string(out), "\n")
}

// execTest runs the tests in the given directory and sets the data
// directory and setup only environment variables.
func execTest(t *testing.T, dir, dataDir string, setupOnly bool) {
cmd := exec.Command("go", "test", ".", "-count", "1", "-v")
cmd.Dir = dir
cmd.Env = append(
os.Environ(),
fmt.Sprintf("%s=%s", enableEnvName, "true"),
fmt.Sprintf("%s=%s", rootDataDirEnvName, dataDir),
)

if setupOnly {
cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", setupOnlyEnvName, "true"))
}

out, err := cmd.Output()
require.NoError(t, err, string(out))
}

// execClone clones the repo from the given url and branch into the directory.
func execClone(t *testing.T, dir, url, branch string) {
cmd := exec.Command(
"git",
"clone",
"--single-branch",
"--branch", branch,
"--depth", "1",
url,
dir,
)

out, err := cmd.Output()
require.NoError(t, err, string(out))
}

// execMakeDeps runs make:deps in the given directory.
func execMakeDeps(t *testing.T, dir string) {
cmd := exec.Command("make", "deps:lens")
cmd.Dir = dir

out, err := cmd.Output()
require.NoError(t, err, string(out))
}
194 changes: 194 additions & 0 deletions tests/change_detector/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
// Copyright 2023 Democratized Data Foundation
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package change_detector

import (
"io/fs"
"os"
"path"
"runtime"
"strconv"
"testing"

"github.com/stretchr/testify/require"
)

var (
// Enabled is true when the change detector is running.
Enabled bool
// SetupOnly is true when the change detector is running in setup mode.
SetupOnly bool
// rootDatabaseDir is the shared database directory for running tests.
rootDatabaseDir string
// repository is the url of the repository to run change detector on.
repository string
// sourceBranch is the name of the source branch to run change detector on.
sourceBranch string
// targetBranch is the name of the target branch to run change detector on.
targetBranch string
// previousTestCaseTestName is the name of the previous test.
previousTestCaseTestName string
)

const (
repositoryEnvName = "DEFRA_CHANGE_DETECTOR_REPOSITORY"
sourceBranchEnvName = "DEFRA_CHANGE_DETECTOR_SOURCE_BRANCH"
targetBranchEnvName = "DEFRA_CHANGE_DETECTOR_TARGET_BRANCH"
setupOnlyEnvName = "DEFRA_CHANGE_DETECTOR_SETUP_ONLY"
rootDataDirEnvName = "DEFRA_CHANGE_DETECTOR_ROOT_DATA_DIR"
enableEnvName = "DEFRA_CHANGE_DETECTOR_ENABLE"
)

const (
defaultRepository = "https://github.com/nasdf/defradb.git"
defaultBranch = "nasdf/test/parallel-change-detector"
documentationDirectoryName = "data_format_changes"
)

func init() {
Enabled, _ = strconv.ParseBool(os.Getenv(enableEnvName))
SetupOnly, _ = strconv.ParseBool(os.Getenv(setupOnlyEnvName))
rootDatabaseDir = os.Getenv(rootDataDirEnvName)

if value, ok := os.LookupEnv(repositoryEnvName); ok {
repository = value
} else {
repository = defaultRepository
}

if value, ok := os.LookupEnv(sourceBranchEnvName); ok {
sourceBranch = value
} else {
sourceBranch = defaultBranch
}

if value, ok := os.LookupEnv(targetBranchEnvName); ok {
targetBranch = value
} else {
targetBranch = defaultBranch
}
}

// DatabaseDir returns the database directory for change detector test.
func DatabaseDir(t testing.TB) string {
return path.Join(rootDatabaseDir, t.Name())
}

// PreTestChecks skips any test that can't be run by the change detector.
func PreTestChecks(t *testing.T, collectionNames []string) {
if !Enabled {
return
}

if previousTestCaseTestName == t.Name() {
t.Skip("skipping duplicate test")
}
previousTestCaseTestName = t.Name()

if len(collectionNames) == 0 {
t.Skip("skipping test with no collections")
}

if SetupOnly {
return
}

_, err := os.Stat(DatabaseDir(t))
if os.IsNotExist(err) {
t.Skip("skipping new test package")
}
require.NoError(t, err)
}

func checkIfDatabaseFormatChangesAreDocumented(t *testing.T, codeDir string) bool {
previousDbChangeFiles, targetDirFound := getDatabaseFormatDocumentation(t, codeDir, false)
if !targetDirFound {
t.Fatalf("Documentation directory not found")
}

previousDbChanges := make(map[string]struct{}, len(previousDbChangeFiles))
for _, f := range previousDbChangeFiles {
// Note: we assume flat directory for now - sub directories are not expanded
previousDbChanges[f.Name()] = struct{}{}
}

_, thisFilePath, _, _ := runtime.Caller(0)
currentDbChanges, currentDirFound := getDatabaseFormatDocumentation(t, thisFilePath, true)
if !currentDirFound {
t.Fatalf("Documentation directory not found")
}

for _, f := range currentDbChanges {
if _, isChangeOld := previousDbChanges[f.Name()]; !isChangeOld {
// If there is a new file in the directory then the change
// has been documented and the test should pass
return true
}
}

return false
}

func getDatabaseFormatDocumentation(t *testing.T, startPath string, allowDescend bool) ([]fs.DirEntry, bool) {
startInfo, err := os.Stat(startPath)
if err != nil {
t.Fatal(err)
}

var currentDirectory string
if startInfo.IsDir() {
currentDirectory = startPath
} else {
currentDirectory = path.Dir(startPath)
}

for {
directoryContents, err := os.ReadDir(currentDirectory)
if err != nil {
t.Fatal(err)
}

for _, directoryItem := range directoryContents {
directoryItemPath := path.Join(currentDirectory, directoryItem.Name())
if directoryItem.Name() == documentationDirectoryName {
probableFormatChangeDirectoryContents, err := os.ReadDir(directoryItemPath)
if err != nil {
t.Fatal(err)
}
for _, possibleDocumentationItem := range probableFormatChangeDirectoryContents {
if path.Ext(possibleDocumentationItem.Name()) == ".md" {
// If the directory's name matches the expected, and contains .md files
// we assume it is the documentation directory
return probableFormatChangeDirectoryContents, true
}
}
} else {
if directoryItem.IsDir() {
childContents, directoryFound := getDatabaseFormatDocumentation(t, directoryItemPath, false)
if directoryFound {
return childContents, true
}
}
}
}

if allowDescend {
// If not found in this directory, continue down the path
currentDirectory = path.Dir(currentDirectory)

if currentDirectory == "." || currentDirectory == "/" {
t.Fatal(err)
}
} else {
return []fs.DirEntry{}, false
}
}
}
Loading