From f2d9bc0972fe78cbe4eb3afea6b634f72fd26be8 Mon Sep 17 00:00:00 2001 From: Steven L Date: Fri, 1 Nov 2024 11:34:15 -0500 Subject: [PATCH 1/3] Fix go-generate calling, do more before running tests Does a couple things: - `make generate` now infers files that might affect generation. This should work as long as we keep simple codegen (the server has moved beyond this, sadly). - `make generate` now forces re-generation regardless of need, because it's a nice workaround for dependency issues like happened in #1373 - `make test` and related targets now go-generate and fmt before running tests. Slower, but you can't forget to re-generate, and they're slow already so it's probably fine. --- Makefile | 13 +++++++------ client/client.go | 2 -- encoded/encoded.go | 2 -- internal/encoded.go | 2 ++ internal/internal_workflow_client.go | 3 +++ 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 689d7e29f..da29eae40 100644 --- a/Makefile +++ b/Makefile @@ -217,7 +217,7 @@ $(THRIFT_GEN): $(THRIFT_FILES) $(BIN)/thriftrw $(BIN)/thriftrw-plugin-yarpc # mockery is quite noisy so it's worth being kinda precise with the files. # this needs to be both the files defining the generate command, AND the files that define the interfaces. -$(BUILD)/generate: client/client.go encoded/encoded.go internal/internal_workflow_client.go internal/internal_public.go internal/internal_task_pollers.go $(BIN)/mockery +$(BUILD)/generate: $(shell grep --files-with-matches -E '^//go:generate' $(ALL_SRC)) $(BIN)/mockery $Q $(BIN_PATH) go generate ./... $Q touch $@ @@ -303,7 +303,8 @@ errcheck: $(BIN)/errcheck $(BUILD)/fmt ## (re)run errcheck $(BIN)/errcheck ./... .PHONY: generate -generate: $(BUILD)/generate ## run go-generate +generate: ## run go-generate (build, lint, fmt all do this for you if needed) + $(call remake,generate) .PHONY: tidy tidy: @@ -372,7 +373,7 @@ UT_DIRS := $(filter-out $(INTEG_TEST_ROOT)%, $(sort $(dir $(filter %_test.go,$(A .PHONY: unit_test integ_test_sticky_off integ_test_sticky_on integ_test_grpc cover test: unit_test integ_test_sticky_off integ_test_sticky_on ## run all tests (requires a running cadence instance) -unit_test: $(ALL_SRC) ## run all unit tests +unit_test: $(BUILD)/fmt ## run all unit tests $Q mkdir -p $(COVER_ROOT) $Q echo "mode: atomic" > $(UT_COVER_FILE) $Q FAIL=""; \ @@ -383,15 +384,15 @@ unit_test: $(ALL_SRC) ## run all unit tests done; test -z "$$FAIL" || (echo "Failed packages; $$FAIL"; exit 1) cat $(UT_COVER_FILE) > .build/cover.out; -integ_test_sticky_off: $(ALL_SRC) +integ_test_sticky_off: $(BUILD)/fmt $Q mkdir -p $(COVER_ROOT) STICKY_OFF=true go test $(TEST_ARG) ./test -coverprofile=$(INTEG_STICKY_OFF_COVER_FILE) -coverpkg=./... -integ_test_sticky_on: $(ALL_SRC) +integ_test_sticky_on: $(BUILD)/fmt $Q mkdir -p $(COVER_ROOT) STICKY_OFF=false go test $(TEST_ARG) ./test -coverprofile=$(INTEG_STICKY_ON_COVER_FILE) -coverpkg=./... -integ_test_grpc: $(ALL_SRC) +integ_test_grpc: $(BUILD)/fmt $Q mkdir -p $(COVER_ROOT) STICKY_OFF=false go test $(TEST_ARG) ./test -coverprofile=$(INTEG_GRPC_COVER_FILE) -coverpkg=./... diff --git a/client/client.go b/client/client.go index f67ff0f88..a614571ea 100644 --- a/client/client.go +++ b/client/client.go @@ -21,8 +21,6 @@ // when adding any, make sure you update the files that it checks in the makefile //go:generate mockery --srcpkg . --name Client --output ../mocks --boilerplate-file ../LICENSE //go:generate mockery --srcpkg . --name DomainClient --output ../mocks --boilerplate-file ../LICENSE -//go:generate mockery --srcpkg go.uber.org/cadence/internal --name HistoryEventIterator --output ../mocks --boilerplate-file ../LICENSE -//go:generate mockery --srcpkg go.uber.org/cadence/internal --name WorkflowRun --output ../mocks --boilerplate-file ../LICENSE // Package client contains functions to create Cadence clients used to communicate to Cadence service. // diff --git a/encoded/encoded.go b/encoded/encoded.go index be5283a1b..0e30282ac 100644 --- a/encoded/encoded.go +++ b/encoded/encoded.go @@ -18,8 +18,6 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -//go:generate mockery --srcpkg go.uber.org/cadence/internal --name Value --output ../mocks --boilerplate-file ../LICENSE - // Package encoded contains wrappers that are used for binary payloads deserialization. package encoded diff --git a/internal/encoded.go b/internal/encoded.go index fd4688a67..3dff2eb00 100644 --- a/internal/encoded.go +++ b/internal/encoded.go @@ -27,6 +27,8 @@ import ( "go.uber.org/cadence/internal/common/util" ) +//go:generate mockery --name Value --output ../mocks --boilerplate-file ../LICENSE + type ( // Value is used to encapsulate/extract encoded value from workflow/activity. Value interface { diff --git a/internal/internal_workflow_client.go b/internal/internal_workflow_client.go index 8021c7744..7618be9b6 100644 --- a/internal/internal_workflow_client.go +++ b/internal/internal_workflow_client.go @@ -41,6 +41,9 @@ import ( "go.uber.org/cadence/internal/common/metrics" ) +//go:generate mockery --name HistoryEventIterator --output ../mocks --boilerplate-file ../LICENSE +//go:generate mockery --name WorkflowRun --output ../mocks --boilerplate-file ../LICENSE + // Assert that structs do indeed implement the interfaces var _ Client = (*workflowClient)(nil) From 4efcc2d4ef8b25ab6734b8ef2ad11c5bc60937a2 Mon Sep 17 00:00:00 2001 From: Steven L Date: Fri, 1 Nov 2024 11:41:35 -0500 Subject: [PATCH 2/3] update comment --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index da29eae40..f7e780d45 100644 --- a/Makefile +++ b/Makefile @@ -216,7 +216,8 @@ $(THRIFT_GEN): $(THRIFT_FILES) $(BIN)/thriftrw $(BIN)/thriftrw-plugin-yarpc $Q touch $@ # mockery is quite noisy so it's worth being kinda precise with the files. -# this needs to be both the files defining the generate command, AND the files that define the interfaces. +# as long as the //go:generate line is in the file that defines the thing to mock, this will auto-discover it. +# if we build any fancier generators, like the server's wrappers, this might need adjusting / switch to completely manual. $(BUILD)/generate: $(shell grep --files-with-matches -E '^//go:generate' $(ALL_SRC)) $(BIN)/mockery $Q $(BIN_PATH) go generate ./... $Q touch $@ From 23961ee97002fd02eb87a110e11befa4149c4988 Mon Sep 17 00:00:00 2001 From: Steven L Date: Fri, 1 Nov 2024 12:12:38 -0500 Subject: [PATCH 3/3] update makefile-doc-comment --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f7e780d45..1f0d1cde2 100644 --- a/Makefile +++ b/Makefile @@ -304,7 +304,7 @@ errcheck: $(BIN)/errcheck $(BUILD)/fmt ## (re)run errcheck $(BIN)/errcheck ./... .PHONY: generate -generate: ## run go-generate (build, lint, fmt all do this for you if needed) +generate: ## run go-generate (build, lint, fmt, tests all do this for you if needed) $(call remake,generate) .PHONY: tidy