feat(ci): add lint, tidy, coverage, and formatting CI jobs

Author: behinddwalls

.github/workflows/ci.yml

@@ -13,39 +13,102 @@ on:
 
 permissions:
   contents: read
-  pull-requests: read
+  pull-requests: write
 
 jobs:
   # ---------------------------------------------------------------------------
-  # BUILD AND UNIT TESTS (special case - Gazelle + build + unit tests)
+  # LINT (all tools from Bazel — gofmt via Go SDK, yamllint via Go binary)
+  # ---------------------------------------------------------------------------
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./.github/actions/setup
+
+      - name: Run linters
+        run: make lint
+
+  # ---------------------------------------------------------------------------
+  # TIDY (module files + BUILD files in sync)
+  # ---------------------------------------------------------------------------
+  tidy:
+    name: Tidy
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./.github/actions/setup
+
+      - name: Check module files are tidy
+        run: make check-tidy
+
+      - name: Check BUILD files are up to date
+        run: make check-gazelle
+
+  # ---------------------------------------------------------------------------
+  # BUILD AND UNIT TESTS
   # ---------------------------------------------------------------------------
   build-and-unit-test:
     name: Build and Unit Test
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
+        with:
+          # Full history needed for diff coverage (git merge-base origin/main HEAD).
+          fetch-depth: 0
       - uses: ./.github/actions/setup
 
-      - name: Check BUILD files are up to date
+      - name: Build project
+        run: make build
+
+      - name: Run unit tests with coverage
+        if: "!contains(github.event.pull_request.labels.*.name, 'COVERAGE_EXEMPTION')"
+        run: make check-coverage
+
+      - name: Run unit tests (coverage exempted)
+        if: contains(github.event.pull_request.labels.*.name, 'COVERAGE_EXEMPTION')
+        run: make coverage
+
+      - name: Coverage summary
+        if: always()
         run: |
-          echo "Running Gazelle to check BUILD files..." >&2
-          make gazelle
-          if ! git diff --quiet; then
-            echo "BUILD files are out of date!" >&2
-            echo "" >&2
-            echo "The following files were modified by Gazelle:" >&2
-            git diff --name-only >&2
-            echo "" >&2
-            echo "Please run 'make gazelle' locally and commit the changes." >&2
-            exit 1
+          if [ -f /tmp/coverage-summary.txt ]; then
+            echo "### Coverage Report" >> "$GITHUB_STEP_SUMMARY"
+            echo '```' >> "$GITHUB_STEP_SUMMARY"
+            cat /tmp/coverage-summary.txt >> "$GITHUB_STEP_SUMMARY"
+            echo '```' >> "$GITHUB_STEP_SUMMARY"
           fi
-          echo "BUILD files are up to date" >&2
 
-      - name: Build project
-        run: make build
+      - name: Notify coverage exemption
+        if: >-
+          github.event_name == 'pull_request' &&
+          contains(github.event.pull_request.labels.*.name, 'COVERAGE_EXEMPTION')
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          SUMMARY=""
+          if [ -f /tmp/coverage-summary.txt ]; then
+            SUMMARY=$(cat /tmp/coverage-summary.txt)
+          fi
+          gh pr comment ${{ github.event.pull_request.number }} --body "$(cat <<EOF
+          **Coverage enforcement exempted** via \`COVERAGE_EXEMPTION\` label.
+
+          \`\`\`
+          ${SUMMARY}
+          \`\`\`
 
-      - name: Run unit tests
-        run: make test || echo "No unit tests found"
+          Reviewers: please verify this exemption is justified.
+          EOF
+          )"
+
+      - name: Upload coverage report
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-report
+          path: coverage-html/
+          if-no-files-found: ignore
+          retention-days: 14
 
   # ---------------------------------------------------------------------------
   # INTEGRATION TESTS (e2e, gateway, orchestrator)
@@ -120,6 +183,8 @@ jobs:
     name: Required Checks
     runs-on: ubuntu-latest
     needs:
+      - lint
+      - tidy
       - build-and-unit-test
       - e2e
       - gateway-integration-test

.gitignore

@@ -12,5 +12,8 @@ MODULE.bazel.lock
 bin/
 .docker-bin/
 
+# Coverage
+coverage-html/
+
 # Make completion cache
 .make_targets_cache

MODULE.bazel

@@ -36,6 +36,7 @@ use_repo(
     "com_github_spf13_cobra",
     "com_github_stretchr_testify",
     "com_github_uber_go_tally_v4",
+    "in_gopkg_yaml_v3",
     "org_golang_google_grpc",
     "org_golang_google_protobuf",
     "org_uber_go_fx",

Makefile

@@ -10,10 +10,46 @@ ORCHESTRATOR_COMPOSE_FILE = example/server/orchestrator/docker-compose.yml
 # Fixed project name for local manual testing (tests use unique random names)
 LOCAL_PROJECT = submitqueue
 
+# Minimum overall coverage percentage (override with: make check-coverage COVERAGE_THRESHOLD=80)
+COVERAGE_THRESHOLD ?= 75
+
+# Minimum diff coverage percentage for new/changed lines (override with: make check-coverage DIFF_COVERAGE_THRESHOLD=90)
+DIFF_COVERAGE_THRESHOLD ?= 85
+
 # Set REPO_ROOT for docker-compose
 export REPO_ROOT := $(shell pwd)
 
-.PHONY: build build-all-linux build-gateway-linux build-orchestrator-linux clean clean-proto deps e2e-test gazelle integration-test integration-test-extensions integration-test-gateway integration-test-orchestrator local-clean local-gateway-start local-gateway-stop local-init-schemas local-logs local-orchestrator-start local-orchestrator-stop local-ps local-restart local-start local-stop proto query-deps query-targets run-client-gateway run-client-orchestrator run-queue-admin test test-no-cache help
+# Runs the coverage tool with optional diff detection.
+# Usage: $(call run_coverage,extra flags...)
+define run_coverage
+	@LCOV=$$($(BAZEL) info output_path)/_coverage/_coverage_report.dat; \
+	MERGE_BASE=$$(git merge-base origin/main HEAD 2>/dev/null || echo ""); \
+	DIFF_ARGS=""; \
+	if [ -n "$$MERGE_BASE" ] && [ "$$(git rev-parse HEAD)" != "$$MERGE_BASE" ]; then \
+		git diff $$MERGE_BASE HEAD > /tmp/coverage-diff.patch; \
+		DIFF_ARGS="-diff /tmp/coverage-diff.patch"; \
+	fi; \
+	$(BAZEL) run //tool/coverage -- \
+		$$DIFF_ARGS \
+		-summary /tmp/coverage-summary.txt \
+		-html $(CURDIR)/coverage-html \
+		-src $(CURDIR) \
+		$(1) \
+		$$LCOV
+endef
+
+# Fails if git working tree is dirty. Usage: $(call assert_clean,fix command)
+define assert_clean
+	@if ! git diff --quiet; then \
+		echo "The following files need updating:" >&2; \
+		git diff --name-only >&2; \
+		echo "" >&2; \
+		echo "Please run '$(1)' locally and commit the changes." >&2; \
+		exit 1; \
+	fi
+endef
+
+.PHONY: build build-all-linux build-gateway-linux build-orchestrator-linux check-coverage check-gazelle check-tidy clean clean-proto coverage deps e2e-test fmt gazelle integration-test integration-test-extensions integration-test-gateway integration-test-orchestrator lint lint-fmt lint-yaml local-clean local-gateway-start local-gateway-stop local-init-schemas local-logs local-orchestrator-start local-orchestrator-stop local-ps local-restart local-start local-stop proto query-deps query-targets run-client-gateway run-client-orchestrator run-queue-admin test test-no-cache tidy tidy-bazel tidy-go help
 
 
 build: ## Build all services and examples
@@ -41,6 +77,21 @@ build-orchestrator-linux: ## Build Orchestrator Linux binary for Docker
 	 cp -f bazel-bin/example/server/orchestrator/orchestrator .docker-bin/orchestrator
 	@echo "Orchestrator Linux binary ready at .docker-bin/orchestrator"
 
+check-coverage: ## Enforce coverage thresholds (overall: $(COVERAGE_THRESHOLD)%, diff: $(DIFF_COVERAGE_THRESHOLD)%)
+	@echo "Running tests with coverage..."
+	@$(BAZEL) coverage //... --test_tag_filters=-manual,-integration --combined_report=lcov
+	$(call run_coverage,-threshold $(COVERAGE_THRESHOLD) -diff-threshold $(DIFF_COVERAGE_THRESHOLD))
+
+check-gazelle: ## Check BUILD.bazel files are up to date
+	@echo "Running Gazelle to check BUILD files..."
+	@$(BAZEL) run //:gazelle
+	$(call assert_clean,make gazelle)
+	@echo "BUILD files are up to date."
+
+check-tidy: tidy ## Check that go.mod and MODULE.bazel are tidy
+	$(call assert_clean,make tidy)
+	@echo "Module files are up to date."
+
 clean: ## Clean generated files and binaries
 	@echo "Cleaning with Bazel..."
 	@$(BAZEL) clean
@@ -53,16 +104,24 @@ clean-proto: ## Clean generated proto files
 	@rm -rf orchestrator/protopb/*.pb.go
 	@echo "Proto clean complete!"
 
-deps: ## Install Go dependencies
-	@echo "Installing Go dependencies..."
-	@go mod download
-	@go mod tidy
+coverage: ## Run tests with coverage, generate HTML report
+	@echo "Running tests with coverage..."
+	@$(BAZEL) coverage //... --test_tag_filters=-manual,-integration --combined_report=lcov
+	$(call run_coverage,)
+
+deps: tidy-go ## Download and tidy Go dependencies
 	@echo "Dependencies installed!"
 
 e2e-test: build-all-linux ## Run end-to-end tests (hermetic, auto-builds binaries)
 	@echo "Running end-to-end tests..."
 	@$(BAZEL) test //test/e2e:e2e_test --test_output=streamed
 
+fmt: ## Format Go code
+	@GOFMT=$$($(BAZEL) run @rules_go//go -- env GOROOT 2>/dev/null | tail -1)/bin/gofmt; \
+	echo "Formatting Go code (using $$GOFMT)..."; \
+	$$GOFMT -w .; \
+	echo "Formatting complete!"
+
 gazelle: ## Update BUILD.bazel files
 	@echo "Running Gazelle to update BUILD files..."
 	@$(BAZEL) run //:gazelle
@@ -83,6 +142,18 @@ integration-test-orchestrator: build-orchestrator-linux ## Run Orchestrator inte
 	@echo "Running Orchestrator integration tests..."
 	@$(BAZEL) test //test/integration/orchestrator:orchestrator_test --test_output=streamed
 
+lint: lint-fmt lint-yaml ## Run all linters
+	@echo "All lint checks passed."
+
+lint-fmt: fmt ## Check Go code formatting (fails if unformatted)
+	$(call assert_clean,make fmt)
+	@echo "All Go files are properly formatted."
+
+lint-yaml: ## Check YAML files for syntax errors
+	@echo "Checking YAML files..."
+	@$(BAZEL) run //tool/linter/yamllint -- .
+	@echo "All YAML files are valid."
+
 local-clean: ## Stop and remove all local services, volumes, and images
 	@echo "Cleaning all services and data..."
 	@$(COMPOSE) -f $(COMPOSE_FILE) -p $(LOCAL_PROJECT) down -v --rmi local
@@ -229,6 +300,16 @@ test-no-cache: ## Run unit tests without cache (force re-run)
 	@echo "Running unit tests (no cache)..."
 	@$(BAZEL) test //... --test_tag_filters=-manual,-integration --nocache_test_results
 
+tidy: tidy-go tidy-bazel ## Run go mod tidy and bazel mod tidy
+
+tidy-bazel: ## Run bazel mod tidy
+	@echo "Running bazel mod tidy..."
+	@$(BAZEL) mod tidy
+
+tidy-go: ## Run go mod tidy
+	@echo "Running go mod tidy..."
+	@$(BAZEL) run @rules_go//go -- mod tidy -e
+
 help: ## Show this help message
 	@echo "Available targets:"
 	@echo ""

core/consumer/registry.go

@@ -57,8 +57,8 @@ type TopicConfig struct {
 // TopicRegistry provides queue, topic name, and subscription config for topics.
 // Each topic can have a different queue backend and topic name.
 type TopicRegistry struct {
-	queues     map[TopicKey]queue.Queue
-	topicNames map[TopicKey]string
+	queues              map[TopicKey]queue.Queue
+	topicNames          map[TopicKey]string
 	subscriptionConfigs map[topicGroup]queue.SubscriptionConfig
 }
 

core/consumer/registry_test.go

@@ -18,8 +18,8 @@ func TestNewTopicRegistry(t *testing.T) {
 	registry, err := consumer.NewTopicRegistry(
 		[]consumer.TopicConfig{
 			{
-				Key:  consumer.TopicKeyRequest,
-				Name: "request",
+				Key:   consumer.TopicKeyRequest,
+				Name:  "request",
 				Queue: mockQ,
 				Subscription: extqueue.DefaultSubscriptionConfig(
 					"worker-1", "group-a",
@@ -44,7 +44,7 @@ func TestNewTopicRegistry(t *testing.T) {
 
 func TestNewTopicRegistry_InvalidTopicName(t *testing.T) {
 	tests := []struct {
-		name string
+		name      string
 		topicName string
 	}{
 		{

entity/build.go

@@ -39,7 +39,6 @@ func (s BuildStatus) IsTerminal() bool {
 	return s == BuildStatusPassed || s == BuildStatusFailed || s == BuildStatusCancelled
 }
 
-
 // SpeculationPathInfo represents the base and head commits of a speculation path used in a build.
 type SpeculationPathInfo struct {
 	// Base is a list of batchIDs(in order) that form the base of this speculation path.

entity/queue/message_test.go

@@ -62,4 +62,3 @@ func TestMessage_Fields(t *testing.T) {
 	assert.Equal(t, msg.PublishedAt, copied.PublishedAt)
 	assert.Equal(t, msg.Metadata, copied.Metadata)
 }
-

entity/request.go

@@ -2,7 +2,6 @@ package entity
 
 import "encoding/json"
 
-
 // RequestLandStrategy defines the possible source control integration methods.
 type RequestLandStrategy string
 

example/server/orchestrator/main.go

@@ -21,14 +21,14 @@ import (
 	githubchecker "github.com/uber/submitqueue/extension/mergechecker/github"
 	extqueue "github.com/uber/submitqueue/extension/queue"
 	queueMySQL "github.com/uber/submitqueue/extension/queue/mysql"
-	mysqlstorage "github.com/uber/submitqueue/extension/storage/mysql"
 	"github.com/uber/submitqueue/extension/storage"
+	mysqlstorage "github.com/uber/submitqueue/extension/storage/mysql"
 	"github.com/uber/submitqueue/orchestrator/controller"
 	"github.com/uber/submitqueue/orchestrator/controller/batch"
 	"github.com/uber/submitqueue/orchestrator/controller/build"
+	"github.com/uber/submitqueue/orchestrator/controller/buildsignal"
 	"github.com/uber/submitqueue/orchestrator/controller/conclude"
 	"github.com/uber/submitqueue/orchestrator/controller/merge"
-	"github.com/uber/submitqueue/orchestrator/controller/buildsignal"
 	"github.com/uber/submitqueue/orchestrator/controller/request"
 	"github.com/uber/submitqueue/orchestrator/controller/score"
 	"github.com/uber/submitqueue/orchestrator/controller/speculate"