Skip to content

Commit 92cf28d

Browse files
authored
feat: e2e testing (#75)
* feat: e2e testing boilerplate * fix: allow custom image and version on e2e tests * refactor: move slack fixtures to testhelper package Prevents test utilities from being compiled and published with the services/slack package while keeping them importable by any test. * fix: address resource leaks, missing validation, and data correctness issues - Terminate orphaned containers on error in Postgres and Mattermost setup - Validate Docker Hub HTTP status before decoding response - Add json:"-" tag to SlackChannel.Type to clarify it is computed - Only set DeleteAt on user export when user is actually deleted * fix: resolve gofmt and staticcheck lint errors * rename: BasicExport -> SlackBasicExport * e2e: added e2e tests * chore: support GO_TEST_FLAGS in `make test` * fix: reset cobra flags between e2e subtests to prevent state leakage Reset all flag values and Changed state on the shared RootCmd before each Execute() call, preventing flag pollution across subtests. Also fix duplicate step numbering in test comments. * fix: correct misleading test name and prevent double-close of zipWriter - Rename test to "handles posts from multiple missing users" since no reactions are tested - Add zipWriterClosed guard in wrapJSONLInZip to prevent double-close when tempFile.Close() fails after zipWriter.Close() succeeds * fix: use network.WithNetworkName for correct Docker network attachment Replace network.WithNetwork with a partial DockerNetwork struct with the direct network.WithNetworkName call. This avoids constructing a DockerNetwork with an empty ID field and is the idiomatic API for attaching a container to an existing network by name. * fix: prevent resource leak on SetupHelper failure Register t.Cleanup(th.TearDown) immediately after creating the TestHelper so that Docker resources are always cleaned up, even if a later setup step (e.g. CreateMattermostContainer) calls require.NoError and aborts before the caller's defer runs. Make TearDown() idempotent by clearing th.tearDowns after execution so it is safe to call from both t.Cleanup and an explicit defer. * fix: stop logging PostgreSQL connection string with credentials The connection string includes username and password, which would be visible in CI logs. Replace with a generic log message. * fix: validate full 4-byte ZIP signature in isZipFile Only checking the first two bytes ("PK") can false-positive on non-ZIP files. Check the full 4-byte magic for local file header (PK\x03\x04), end of central directory (PK\x05\x06), and spanned archive (PK\x07\x08). * fix: normalize ZIP entry paths to forward slashes filepath.Rel returns OS-specific separators (backslashes on Windows). The ZIP specification requires forward slashes for entry paths. Add filepath.ToSlash after filepath.Rel to ensure compliance. * fix: use crypto/rand for unique team names to prevent CI collisions time.Now().UnixNano()%10000 has only 4 digits of entropy and can collide when parallel CI jobs run simultaneously. Replace with crypto/rand hex suffix for reliable uniqueness. * refactor: rename importJobMaxAttempts to importJobTimeout The constant is used as a deadline for time.After, not as an attempt count. With exponential backoff only ~8-10 polls actually occur. Rename to importJobTimeout with an explicit time.Duration value and inline the initial poll interval to make the intent clearer. * fix: use consistent Makefile variable syntax Replace ${GO_TEST_FLAGS} with $(GO_TEST_FLAGS) to match the $(...) convention used everywhere else in the Makefile. * refactor: cache allChannels() result in validate() allChannels() was called twice, allocating two identical slices. Store the result in a local variable and reuse it. * refactor: consolidate transform-slack.log cleanup Extract the log filename to a constant and use a single t.Cleanup in each parent test function instead of repeating defer os.Remove("transform-slack.log") in every subtest. * fix: document why MM_SERVICESETTINGS_SITEURL uses internal address The SiteURL env var is set to the container-internal address because the host-mapped port is not known until after the container starts. Add a comment explaining this is intentional and only affects Mattermost's self-referential links, not the test API client. * fix: add page limit to Docker Hub tag pagination loop Guard against infinite pagination in fetchLatestStableTag by capping at 5 pages (500 tags). This prevents the loop from running forever if the Docker Hub API returns a malformed or circular next link. * fix: remove unnecessary log from TestFetchLatestStableTag The resolved tag is already validated by assertions; logging it adds noise without aiding debugging. * fix: guard against path traversal via channel names in SlackExportBuilder A malformed channel name like "../../etc" would cause the export builder to write files outside the temporary directory. Validate that the resolved channel directory stays within the temp dir. * fix: remove redundant defer reader.Close() in ParseSlackExportFile The reader opened via os.Open is assigned to the same variable that already has a defer reader.Close() registered at line 366. The extra defer inside the USERS_JSON_FILE branch is redundant and misleading. * fix: apply gofmt formatting to containers.go Remove extra alignment whitespace in env map literals that was causing golangci-lint gofmt check failures in CI. * fix: auto-download pinned golangci-lint and handle rand.Read error Manage golangci-lint locally in tools/bin/ pinned to v2.10.1, downloaded automatically from GitHub releases instead of requiring a global install. Also add a time-based fallback in uniqueTeamName when crypto/rand fails. * add AGENTS.md with post-change checklist for CI agents
1 parent 1f79651 commit 92cf28d

File tree

15 files changed

+3444
-382
lines changed

15 files changed

+3444
-382
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,8 @@ tmp
1313
# IDE and editor files
1414
.idea
1515

16+
# Locally installed tools
17+
tools/
18+
1619
# OS files
1720
.DS_Store

AGENTS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Agents
2+
3+
## After making code changes
4+
5+
Always run the following before considering work complete:
6+
7+
1. `make check-style` — linting and style checks
8+
2. `make test` — full test suite

Makefile

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,24 @@ BUILD_VERSION ?= $(shell git ls-remote --tags --refs https://github.com/mattermo
88
LDFLAGS += -X "github.com/mattermost/mmetl/commands.BuildHash=$(BUILD_HASH)"
99
LDFLAGS += -X "github.com/mattermost/mmetl/commands.Version=$(BUILD_VERSION)"
1010
BUILD_COMMAND ?= go build -ldflags '$(LDFLAGS)'
11+
GO_TEST_FLAGS ?=
12+
13+
GOLANGCI_LINT_VERSION ?= 2.10.1
14+
TOOLS_BIN := $(shell pwd)/tools/bin
15+
GOLANGCI_LINT := $(TOOLS_BIN)/golangci-lint
16+
17+
UNAME_S := $(shell uname -s)
18+
UNAME_M := $(shell uname -m)
19+
ifeq ($(UNAME_S),Linux)
20+
GOLANGCI_LINT_OS := linux
21+
else ifeq ($(UNAME_S),Darwin)
22+
GOLANGCI_LINT_OS := darwin
23+
endif
24+
ifeq ($(UNAME_M),x86_64)
25+
GOLANGCI_LINT_ARCH := amd64
26+
else ifneq (,$(filter $(UNAME_M),arm64 aarch64))
27+
GOLANGCI_LINT_ARCH := arm64
28+
endif
1129

1230
all: build
1331

@@ -43,15 +61,17 @@ package: check-style
4361

4462
rm mmetl mmetl.exe
4563

46-
golangci-lint:
47-
# https://stackoverflow.com/a/677212/1027058 (check if a command exists or not)
48-
@if ! [ -x "$$(command -v golangci-lint)" ]; then \
49-
echo "golangci-lint is not installed. Please see https://github.com/golangci/golangci-lint#install for installation instructions."; \
50-
exit 1; \
51-
fi; \
64+
$(TOOLS_BIN)/golangci-lint-v$(GOLANGCI_LINT_VERSION):
65+
@mkdir -p $(TOOLS_BIN)
66+
@echo "Downloading golangci-lint v$(GOLANGCI_LINT_VERSION)..."
67+
@rm -f $(TOOLS_BIN)/golangci-lint-v* $(TOOLS_BIN)/golangci-lint
68+
@curl -sSfL "https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/golangci-lint-$(GOLANGCI_LINT_VERSION)-$(GOLANGCI_LINT_OS)-$(GOLANGCI_LINT_ARCH).tar.gz" | tar xz -C $(TOOLS_BIN) --strip-components=1 "golangci-lint-$(GOLANGCI_LINT_VERSION)-$(GOLANGCI_LINT_OS)-$(GOLANGCI_LINT_ARCH)/golangci-lint"
69+
@mv $(TOOLS_BIN)/golangci-lint $(TOOLS_BIN)/golangci-lint-v$(GOLANGCI_LINT_VERSION)
70+
@ln -sf golangci-lint-v$(GOLANGCI_LINT_VERSION) $(TOOLS_BIN)/golangci-lint
5271

72+
golangci-lint: $(TOOLS_BIN)/golangci-lint-v$(GOLANGCI_LINT_VERSION)
5373
@echo Running golangci-lint
54-
golangci-lint run ./...
74+
$(GOLANGCI_LINT) run ./...
5575

5676
gofmt:
5777
@echo Running gofmt
@@ -71,7 +91,7 @@ gofmt:
7191

7292
test:
7393
@echo Running tests
74-
$(GO) test -race -v $(GO_PACKAGES) -count=1
94+
$(GO) test -race -v $(GO_PACKAGES) -count=1 $(GO_TEST_FLAGS)
7595

7696
check-style: golangci-lint
7797

0 commit comments

Comments
 (0)