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.

Sign up for GitHub

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

Jump to bottom

fix: GitHub tests emitting unexpected keepers = {} #4462

Merged
merged 3 commits into from
Apr 24, 2024
Merged

fix: GitHub tests emitting unexpected keepers = {} #4462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2a24a0
fix: github tests emitting unexpected keepers = {}
jippi Apr 19, 2024
b5c66ee
fix: run tests when golden files are changed as well
jippi Apr 19, 2024
ab9c878
fix: install goimports before using it, in CI
jippi Apr 19, 2024
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
2 changes: 2 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
filters: |
go:
- '**.go'
- '**.txt' # golden file test output
- 'go.*'
- '.github/workflows/test.yml'
test:
Expand All @@ -51,6 +52,7 @@ jobs:

- run: make test-all
- run: make check-fmt

###########################################################
# Notifying #contributors about test failure on main branch
###########################################################
Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ check-lint: ## Run linter in CI/CD. If running locally use 'lint'

.PHONY: check-fmt
check-fmt: ## Fail if not formatted
if [[ $$(goimports -l $$(find . -type f -name '*.go' ! -path "./vendor/*" ! -path "**/mocks/*")) ]]; then exit 1; fi
./scripts/fmt.sh

.PHONY: end-to-end-deps
end-to-end-deps: ## Install e2e dependencies
Expand Down
23 changes: 23 additions & 0 deletions scripts/fmt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/usr/bin/env bash

set -euo pipefail

go install golang.org/x/tools/cmd/goimports@latest

gobin="$(go env GOPATH)/bin"
declare -r gobin

declare -a files
readarray -d '' files < <(find . -type f -name '*.go' ! -name 'mock_*' ! -path './vendor/*' ! -path '**/mocks/*' -print0)
declare -r files

output="$("${gobin}"/goimports -l "${files[@]}")"
declare -r output

if [[ -n "$output" ]]; then
echo "These files had their 'import' changed - please fix them locally and push a fix"
Copy link
Member

Choose a reason for hiding this comment

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

In the future, we may be able to automate this via pre-commit-config


echo "$output"

exit 1
fi
1 change: 0 additions & 1 deletion server/controllers/events/testdata/test-repos/import-multiple-project/dir1/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
resource "random_id" "dummy1" {
Copy link
Member

@nitrocode nitrocode Apr 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since this issue came from a provider version update, to keep the tests deterministic, what do you think of pinning the provider versions for each of these terraform random provider (and possibly other provider) tests?

We can add it to main.tf or add a new versions.tf like upstream modules do it.

Suggested change
resource "random_id" "dummy1" {
provider "random" {}
terraform {
required_providers {
random = {
source = "hashicorp/random"
version = "= 3.6.1"
}
}
}
resource "random_id" "dummy1" {

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree we should probably pin this, or at least worth a discussion. I'm fine making that a followup and merging this as-is, since unit tests being broken on main is causing downstream issues.

keepers = {}
byte_length = 1
}
1 change: 0 additions & 1 deletion server/controllers/events/testdata/test-repos/import-multiple-project/dir2/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
resource "random_id" "dummy2" {
Copy link
Member

Choose a reason for hiding this comment

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

Related https://github.com/runatlantis/atlantis/pull/4462/files#r1573148934

Suggested change
resource "random_id" "dummy2" {
terraform {
required_providers {
random = {
source = "hashicorp/random"
version = "= 3.6.1"
}
}
}
resource "random_id" "dummy2" {

keepers = {}
byte_length = 1
}
4 changes: 1 addition & 3 deletions ...er/controllers/events/testdata/test-repos/import-multiple-project/exp-output-autoplan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -59,7 +58,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -89,4 +87,4 @@ Plan: 1 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
3 changes: 1 addition & 2 deletions .../controllers/events/testdata/test-repos/import-multiple-project/exp-output-plan-again.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -73,4 +72,4 @@ Plan: 1 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
4 changes: 1 addition & 3 deletions .../controllers/events/testdata/test-repos/import-single-project-var/exp-output-autoplan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

# random_id.for_each["default"] will be created
Expand All @@ -28,7 +27,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 2 to add, 0 to change, 0 to destroy.
Expand All @@ -54,4 +52,4 @@ Plan: 2 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
2 changes: 0 additions & 2 deletions server/controllers/events/testdata/test-repos/import-single-project-var/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
resource "random_id" "for_each" {
for_each = toset([var.var])
keepers = {}
byte_length = 1
}

resource "random_id" "count" {
count = 1
keepers = {}
byte_length = 1
}

Expand Down
4 changes: 1 addition & 3 deletions server/controllers/events/testdata/test-repos/import-single-project/exp-output-autoplan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

# random_id.dummy2 will be created
Expand All @@ -28,7 +27,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 2 to add, 0 to change, 0 to destroy.
Expand All @@ -54,4 +52,4 @@ Plan: 2 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
2 changes: 0 additions & 2 deletions server/controllers/events/testdata/test-repos/import-single-project/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
resource "random_id" "dummy1" {
keepers = {}
byte_length = 1
}

resource "random_id" "dummy2" {
keepers = {}
byte_length = 1
}
2 changes: 0 additions & 2 deletions server/controllers/events/testdata/test-repos/import-workspace/dir1/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
resource "random_id" "dummy1" {
count = terraform.workspace == "ops" ? 1 : 0

keepers = {}
byte_length = 1
}

resource "random_id" "dummy2" {
count = terraform.workspace == "ops" ? 1 : 0

keepers = {}
byte_length = 1
}

Expand Down
1 change: 0 additions & 1 deletion server/controllers/events/testdata/test-repos/state-rm-multiple-project/dir1/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
resource "random_id" "dummy" {
keepers = {}
byte_length = 1
}
1 change: 0 additions & 1 deletion server/controllers/events/testdata/test-repos/state-rm-multiple-project/dir2/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
resource "random_id" "dummy" {
keepers = {}
byte_length = 1
}
4 changes: 1 addition & 3 deletions .../controllers/events/testdata/test-repos/state-rm-multiple-project/exp-output-autoplan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -59,7 +58,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -89,4 +87,4 @@ Plan: 1 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
4 changes: 1 addition & 3 deletions ...ontrollers/events/testdata/test-repos/state-rm-multiple-project/exp-output-plan-again.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -59,7 +58,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand Down Expand Up @@ -89,4 +87,4 @@ Plan: 1 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
5 changes: 1 addition & 4 deletions ...er/controllers/events/testdata/test-repos/state-rm-single-project/exp-output-autoplan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

# random_id.for_each["default"] will be created
Expand All @@ -28,7 +27,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

# random_id.simple will be created
Expand All @@ -39,7 +37,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 3 to add, 0 to change, 0 to destroy.
Expand All @@ -65,4 +62,4 @@ Plan: 3 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
5 changes: 1 addition & 4 deletions .../controllers/events/testdata/test-repos/state-rm-single-project/exp-output-plan-again.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

# random_id.for_each["overridden"] will be created
Expand All @@ -28,7 +27,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

# random_id.simple will be created
Expand All @@ -39,7 +37,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 3 to add, 0 to change, 0 to destroy.
Expand All @@ -65,4 +62,4 @@ Plan: 3 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```
3 changes: 0 additions & 3 deletions server/controllers/events/testdata/test-repos/state-rm-single-project/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
resource "random_id" "simple" {
keepers = {}
byte_length = 1
}

resource "random_id" "for_each" {
for_each = toset([var.var])
keepers = {}
byte_length = 1
}

resource "random_id" "count" {
count = 1
keepers = {}
byte_length = 1
}

Expand Down
1 change: 0 additions & 1 deletion server/controllers/events/testdata/test-repos/state-rm-workspace/dir1/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
resource "random_id" "dummy1" {
count = terraform.workspace == "ops" ? 1 : 0

keepers = {}
byte_length = 1
}

Expand Down
3 changes: 1 addition & 2 deletions server/controllers/events/testdata/test-repos/state-rm-workspace/exp-output-plan-again.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Terraform will perform the following actions:
+ dec = (known after apply)
+ hex = (known after apply)
+ id = (known after apply)
+ keepers = {}
}

Plan: 1 to add, 0 to change, 0 to destroy.
Expand All @@ -43,4 +42,4 @@ Plan: 1 to add, 0 to change, 0 to destroy.
* :put_litter_in_its_place: To **delete** all plans and locks from this Pull Request, comment:
```shell
atlantis unlock
```
```