From 36b817344f3173ed50a5b6e194d3896146e53f13 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 26 Jan 2023 15:43:22 -0800 Subject: [PATCH 1/8] Add clearer directions for custom test suite vars in Makefile. --- Makefile | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 566c4de9e4d..afce2abcc10 100644 --- a/Makefile +++ b/Makefile @@ -8,16 +8,30 @@ endif LOGS_DIR := ./logs -# Optional flag to invoke tests using our CI env. -# But we always want these active for structured -# log testing. -CI_FLAGS =\ - DBT_TEST_USER_1=dbt_test_user_1\ - DBT_TEST_USER_2=dbt_test_user_2\ - DBT_TEST_USER_3=dbt_test_user_3\ - RUSTFLAGS="-D warnings"\ - LOG_DIR=./logs\ - DBT_LOG_FORMAT=json +# Flags that dbt Labs uses in our CI env. To override +# these, preface the `make` command on your command +# line with CI_FLAGS=false. Then, uncomment the +# CUSTOM_CI_FLAGS variable and fill with values that +# fit your test suite's needs. +USE_DEFAULT_CI_FLAGS ?= true + +DEFAULT_CI_FLAGS =\ + DBT_TEST_USER_1=dbt_test_user_1\ + DBT_TEST_USER_2=dbt_test_user_2\ + DBT_TEST_USER_3=dbt_test_user_3\ + RUSTFLAGS="-D warnings"\ + LOG_DIR =./logs\ + DBT_LOG_FORMAT=json + +# CUSTOM_CI_FLAGS =\ +# DBT_TEST_USER_1=\ +# DBT_TEST_USER_2=\ +# DBT_TEST_USER_3=\ +# RUSTFLAGS=\ +# LOG_DIR =\ +# DBT_LOG_FORMAT= + + .PHONY: dev_req dev_req: ## Installs dbt-* packages in develop mode along with only development dependencies. @@ -66,7 +80,7 @@ test: .env ## Runs unit tests with py and code checks against staged changes. .PHONY: integration integration: .env ## Runs postgres integration tests with py-integration @\ - $(if $(USE_CI_FLAGS), $(CI_FLAGS)) $(DOCKER_CMD) tox -e py-integration -- -nauto + $(if $(filter true,$(USE_DEFAULT_CI_FLAGS)), $(DEFAULT_CI_FLAGS), $(CUSTOM_CI_FLAGS)) $(DOCKER_CMD) tox -e py-integration -- -nauto .PHONY: integration-fail-fast integration-fail-fast: .env ## Runs postgres integration tests with py-integration in "fail fast" mode. @@ -77,7 +91,7 @@ integration-fail-fast: .env ## Runs postgres integration tests with py-integrati interop: clean @\ mkdir $(LOGS_DIR) && \ - $(CI_FLAGS) $(DOCKER_CMD) tox -e py-integration -- -nauto && \ + $(DEFAULT_CI_FLAGS) $(DOCKER_CMD) tox -e py-integration -- -nauto && \ LOG_DIR=$(LOGS_DIR) cargo run --manifest-path test/interop/log_parsing/Cargo.toml .PHONY: setup-db From 178edd82a4fa7d9a0f6569a233670e3453f861c6 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 26 Jan 2023 15:47:27 -0800 Subject: [PATCH 2/8] Fix up PR for review --- .../unreleased/Features-20230126-154716.yaml | 6 +++++ Makefile | 24 +++++++++---------- 2 files changed, 18 insertions(+), 12 deletions(-) create mode 100644 .changes/unreleased/Features-20230126-154716.yaml diff --git a/.changes/unreleased/Features-20230126-154716.yaml b/.changes/unreleased/Features-20230126-154716.yaml new file mode 100644 index 00000000000..4b0bbea2be8 --- /dev/null +++ b/.changes/unreleased/Features-20230126-154716.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Adjust makefile to have clearer instructions for CI env var changes. +time: 2023-01-26T15:47:16.887327-08:00 +custom: + Author: versusfacit + Issue: "6689" diff --git a/Makefile b/Makefile index afce2abcc10..c38b0a42c9d 100644 --- a/Makefile +++ b/Makefile @@ -16,20 +16,20 @@ LOGS_DIR := ./logs USE_DEFAULT_CI_FLAGS ?= true DEFAULT_CI_FLAGS =\ - DBT_TEST_USER_1=dbt_test_user_1\ - DBT_TEST_USER_2=dbt_test_user_2\ - DBT_TEST_USER_3=dbt_test_user_3\ - RUSTFLAGS="-D warnings"\ - LOG_DIR =./logs\ - DBT_LOG_FORMAT=json + DBT_TEST_USER_1=dbt_test_user_1\ + DBT_TEST_USER_2=dbt_test_user_2\ + DBT_TEST_USER_3=dbt_test_user_3\ + RUSTFLAGS="-D warnings"\ + LOG_DIR =./logs\ + DBT_LOG_FORMAT=json # CUSTOM_CI_FLAGS =\ -# DBT_TEST_USER_1=\ -# DBT_TEST_USER_2=\ -# DBT_TEST_USER_3=\ -# RUSTFLAGS=\ -# LOG_DIR =\ -# DBT_LOG_FORMAT= + DBT_TEST_USER_1=\ + DBT_TEST_USER_2=\ + DBT_TEST_USER_3=\ + RUSTFLAGS=\ + LOG_DIR =\ + DBT_LOG_FORMAT= From 9c1ac3963ddece522906bcad35c971d126b62dbb Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 26 Jan 2023 15:50:52 -0800 Subject: [PATCH 3/8] Fix erroneous whitespace. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index c38b0a42c9d..c69dc0dc50e 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ DEFAULT_CI_FLAGS =\ DBT_TEST_USER_2=dbt_test_user_2\ DBT_TEST_USER_3=dbt_test_user_3\ RUSTFLAGS="-D warnings"\ - LOG_DIR =./logs\ + LOG_DIR=./logs\ DBT_LOG_FORMAT=json # CUSTOM_CI_FLAGS =\ @@ -28,7 +28,7 @@ DEFAULT_CI_FLAGS =\ DBT_TEST_USER_2=\ DBT_TEST_USER_3=\ RUSTFLAGS=\ - LOG_DIR =\ + LOG_DIR=\ DBT_LOG_FORMAT= From 638e43f9c3283f12d96cc6330e6a1091a3915b9d Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 26 Jan 2023 15:56:12 -0800 Subject: [PATCH 4/8] Fix a spelling error. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index c69dc0dc50e..a066b1dd305 100644 --- a/Makefile +++ b/Makefile @@ -10,8 +10,8 @@ LOGS_DIR := ./logs # Flags that dbt Labs uses in our CI env. To override # these, preface the `make` command on your command -# line with CI_FLAGS=false. Then, uncomment the -# CUSTOM_CI_FLAGS variable and fill with values that +# line with USE_DEFAULT_CI_FLAGS=false. Then, uncomment +# the CUSTOM_CI_FLAGS variable and fill with values that # fit your test suite's needs. USE_DEFAULT_CI_FLAGS ?= true From fa423a4cecf7c1aecd8cd51c85a6cbdc58c30af2 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Fri, 27 Jan 2023 14:35:30 -0800 Subject: [PATCH 5/8] Add documentation to discourage makefile edits but provide override tooling. --- .gitignore | 1 + CONTRIBUTING.md | 15 +++++++++++++++ Makefile | 51 +++++++++++++++++++++++-------------------------- 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/.gitignore b/.gitignore index dc9996305d3..8360802f09b 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,7 @@ coverage.xml *,cover .hypothesis/ test.env +makefile.test.env *.pytest_cache/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 893979fd9ac..a9d89e216b9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,6 +155,21 @@ make integration > These make targets assume you have a local installation of a recent version of [`tox`](https://tox.readthedocs.io/en/latest/) for unit/integration testing and pre-commit for code quality checks, > unless you use choose a Docker container to run tests. Run `make help` for more info. +To override variables used in Makefile targets, create `makefile.test.env` and +define any (or each) variable as your test suite requires: +``` +cat > makefile.test.env << EOF +DBT_TEST_USER_1= +DBT_TEST_USER_2= +DBT_TEST_USER_3= +RUSTFLAGS= +LOG_DIR= +DBT_LOG_FORMAT= +EOF +``` + +Note: This `makefile.test.env` file is git-ignored, but do not check this into the repo. + Check out the other targets in the Makefile to see other commonly used test suites. diff --git a/Makefile b/Makefile index a066b1dd305..18f21c51197 100644 --- a/Makefile +++ b/Makefile @@ -6,31 +6,28 @@ ifeq ($(USE_DOCKER),true) DOCKER_CMD := docker-compose run --rm test endif -LOGS_DIR := ./logs - -# Flags that dbt Labs uses in our CI env. To override -# these, preface the `make` command on your command -# line with USE_DEFAULT_CI_FLAGS=false. Then, uncomment -# the CUSTOM_CI_FLAGS variable and fill with values that -# fit your test suite's needs. -USE_DEFAULT_CI_FLAGS ?= true - -DEFAULT_CI_FLAGS =\ - DBT_TEST_USER_1=dbt_test_user_1\ - DBT_TEST_USER_2=dbt_test_user_2\ - DBT_TEST_USER_3=dbt_test_user_3\ - RUSTFLAGS="-D warnings"\ - LOG_DIR=./logs\ - DBT_LOG_FORMAT=json +# +# Set or override CI flags +# +ifeq (./makefile.test.env,$(wildcard ./makefile.test.env)) + include ./makefile.test.env +endif -# CUSTOM_CI_FLAGS =\ - DBT_TEST_USER_1=\ - DBT_TEST_USER_2=\ - DBT_TEST_USER_3=\ - RUSTFLAGS=\ - LOG_DIR=\ - DBT_LOG_FORMAT= +# DO NOT edit. Override in a makefile.test.env. See CONTRIBUTING.md +DBT_TEST_USER_1 := $(if $(DBT_TEST_USER_1),$(DBT_TEST_USER_1),dbt_test_user_1) +DBT_TEST_USER_2 := $(if $(DBT_TEST_USER_2),$(DBT_TEST_USER_2),dbt_test_user_2) +DBT_TEST_USER_3 := $(if $(DBT_TEST_USER_3),$(DBT_TEST_USER_3),dbt_test_user_3) +RUSTFLAGS := $(if $(RUSTFLAGS),$(RUSTFLAGS),\"-D warnings\") +LOG_DIR := $(if $(LOG_DIR),$(LOG_DIR),./logs) +DBT_LOG_FORMAT := $(if $(DBT_LOG_FORMAT),$(DBT_LOG_FORMAT),json) +CI_FLAGS =\ + DBT_TEST_USER_1=$(DBT_TEST_USER_1)\ + DBT_TEST_USER_2=$(DBT_TEST_USER_2)\ + DBT_TEST_USER_3=$(DBT_TEST_USER_3)\ + RUSTFLAGS=$(RUSTFLAGS)\ + LOG_DIR=$(LOG_DIR)\ + DBT_LOG_FORMAT=$(DBT_LOG_FORMAT) .PHONY: dev_req @@ -80,7 +77,7 @@ test: .env ## Runs unit tests with py and code checks against staged changes. .PHONY: integration integration: .env ## Runs postgres integration tests with py-integration @\ - $(if $(filter true,$(USE_DEFAULT_CI_FLAGS)), $(DEFAULT_CI_FLAGS), $(CUSTOM_CI_FLAGS)) $(DOCKER_CMD) tox -e py-integration -- -nauto + $(CI_FLAGS) $(DOCKER_CMD) tox -e py-integration -- -nauto .PHONY: integration-fail-fast integration-fail-fast: .env ## Runs postgres integration tests with py-integration in "fail fast" mode. @@ -90,9 +87,9 @@ integration-fail-fast: .env ## Runs postgres integration tests with py-integrati .PHONY: interop interop: clean @\ - mkdir $(LOGS_DIR) && \ - $(DEFAULT_CI_FLAGS) $(DOCKER_CMD) tox -e py-integration -- -nauto && \ - LOG_DIR=$(LOGS_DIR) cargo run --manifest-path test/interop/log_parsing/Cargo.toml + mkdir $(LOG_DIR) && \ + $(CI_FLAGS) $(DOCKER_CMD) tox -e py-integration -- -nauto && \ + LOG_DIR=$(LOG_DIR) cargo run --manifest-path test/interop/log_parsing/Cargo.toml .PHONY: setup-db setup-db: ## Setup Postgres database with docker-compose for system testing. From 0304b61e7f323016e68602c4d7aceba6dca2e8de Mon Sep 17 00:00:00 2001 From: Mila Page Date: Fri, 27 Jan 2023 14:40:41 -0800 Subject: [PATCH 6/8] Fix quotation marks. Very strange behavior --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 18f21c51197..861c2c97320 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ endif DBT_TEST_USER_1 := $(if $(DBT_TEST_USER_1),$(DBT_TEST_USER_1),dbt_test_user_1) DBT_TEST_USER_2 := $(if $(DBT_TEST_USER_2),$(DBT_TEST_USER_2),dbt_test_user_2) DBT_TEST_USER_3 := $(if $(DBT_TEST_USER_3),$(DBT_TEST_USER_3),dbt_test_user_3) -RUSTFLAGS := $(if $(RUSTFLAGS),$(RUSTFLAGS),\"-D warnings\") +RUSTFLAGS := $(if $(RUSTFLAGS),$(RUSTFLAGS),-D warnings) LOG_DIR := $(if $(LOG_DIR),$(LOG_DIR),./logs) DBT_LOG_FORMAT := $(if $(DBT_LOG_FORMAT),$(DBT_LOG_FORMAT),json) @@ -25,7 +25,7 @@ CI_FLAGS =\ DBT_TEST_USER_1=$(DBT_TEST_USER_1)\ DBT_TEST_USER_2=$(DBT_TEST_USER_2)\ DBT_TEST_USER_3=$(DBT_TEST_USER_3)\ - RUSTFLAGS=$(RUSTFLAGS)\ + RUSTFLAGS="$(RUSTFLAGS)"\ LOG_DIR=$(LOG_DIR)\ DBT_LOG_FORMAT=$(DBT_LOG_FORMAT) From ffb3e422a5c7040bed4a9b70e673c64118150a43 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Fri, 27 Jan 2023 16:53:10 -0800 Subject: [PATCH 7/8] Compact code and verify quotations happy inside bash and python. --- Makefile | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 861c2c97320..85028c9e206 100644 --- a/Makefile +++ b/Makefile @@ -7,27 +7,19 @@ ifeq ($(USE_DOCKER),true) endif # -# Set or override CI flags +# Set CI flags. Do not edit. Override in a makefile.text.env. See CONTRIBUTING.md # ifeq (./makefile.test.env,$(wildcard ./makefile.test.env)) include ./makefile.test.env endif -# DO NOT edit. Override in a makefile.test.env. See CONTRIBUTING.md -DBT_TEST_USER_1 := $(if $(DBT_TEST_USER_1),$(DBT_TEST_USER_1),dbt_test_user_1) -DBT_TEST_USER_2 := $(if $(DBT_TEST_USER_2),$(DBT_TEST_USER_2),dbt_test_user_2) -DBT_TEST_USER_3 := $(if $(DBT_TEST_USER_3),$(DBT_TEST_USER_3),dbt_test_user_3) -RUSTFLAGS := $(if $(RUSTFLAGS),$(RUSTFLAGS),-D warnings) -LOG_DIR := $(if $(LOG_DIR),$(LOG_DIR),./logs) -DBT_LOG_FORMAT := $(if $(DBT_LOG_FORMAT),$(DBT_LOG_FORMAT),json) - CI_FLAGS =\ - DBT_TEST_USER_1=$(DBT_TEST_USER_1)\ - DBT_TEST_USER_2=$(DBT_TEST_USER_2)\ - DBT_TEST_USER_3=$(DBT_TEST_USER_3)\ - RUSTFLAGS="$(RUSTFLAGS)"\ - LOG_DIR=$(LOG_DIR)\ - DBT_LOG_FORMAT=$(DBT_LOG_FORMAT) + DBT_TEST_USER_1=$(if $(DBT_TEST_USER_1),$(DBT_TEST_USER_1),dbt_test_user_1)\ + DBT_TEST_USER_2=$(if $(DBT_TEST_USER_2),$(DBT_TEST_USER_2),dbt_test_user_2)\ + DBT_TEST_USER_3=$(if $(DBT_TEST_USER_3),$(DBT_TEST_USER_3),dbt_test_user_3)\ + RUSTFLAGS=$(if $(RUSTFLAGS),$(RUSTFLAGS),"-D warnings")\ + LOG_DIR=$(if $(LOG_DIR),$(LOG_DIR),./logs)\ + DBT_LOG_FORMAT=$(if $(DBT_LOG_FORMAT),$(DBT_LOG_FORMAT),json) .PHONY: dev_req From 7e2cbb925417445ac5782c7a2826b097f1205a3c Mon Sep 17 00:00:00 2001 From: Mila Page Date: Tue, 31 Jan 2023 12:45:38 -0800 Subject: [PATCH 8/8] Fold comments into Makefile. --- CONTRIBUTING.md | 15 --------------- Makefile | 7 ++++++- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a9d89e216b9..893979fd9ac 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,21 +155,6 @@ make integration > These make targets assume you have a local installation of a recent version of [`tox`](https://tox.readthedocs.io/en/latest/) for unit/integration testing and pre-commit for code quality checks, > unless you use choose a Docker container to run tests. Run `make help` for more info. -To override variables used in Makefile targets, create `makefile.test.env` and -define any (or each) variable as your test suite requires: -``` -cat > makefile.test.env << EOF -DBT_TEST_USER_1= -DBT_TEST_USER_2= -DBT_TEST_USER_3= -RUSTFLAGS= -LOG_DIR= -DBT_LOG_FORMAT= -EOF -``` - -Note: This `makefile.test.env` file is git-ignored, but do not check this into the repo. - Check out the other targets in the Makefile to see other commonly used test suites. diff --git a/Makefile b/Makefile index 85028c9e206..62ee6f66e8c 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,12 @@ ifeq ($(USE_DOCKER),true) endif # -# Set CI flags. Do not edit. Override in a makefile.text.env. See CONTRIBUTING.md +# To override CI_flags, create a file at this repo's root dir named `makefile.test.env`. Fill it +# with any ENV_VAR overrides required by your test environment, e.g. +# DBT_TEST_USER_1=user +# LOG_DIR="dir with a space in it" +# +# Warn: Restrict each line to one variable only. # ifeq (./makefile.test.env,$(wildcard ./makefile.test.env)) include ./makefile.test.env