Fix project approval rule tests

[armsnyder]

Fixes a failing test (see #659), only touching test code.

The main fix was:

• Explicitly create new protected branches to use in the test, instead of relying on the test project's default branch.
• Use the branch protection ID instead of the branch name when passed to the protected_branch_ids attribute of gitlab_project_approval_rule.

As a follow-up later we can add a computed protected_branch_id attribute to gitlab_branch_protection. This PR does not do that.

Anticipated questions

Why add Gomega?

Gomega is a matcher package used by Ginkgo, a BDD framework which we are NOT using. I wanted a matcher library in order to make assertions about slices containing elements. Gomega was the only package I could find which has the ability to convert assertion failures to errors, which is needed in order to be useful in a TestCheckFunc. I imagine Gomega being useful in the future too, as it reduces boilerplate in our ...ExpectedAttributes test functions.

Why lift the user/group/memberships out of the Terraform test config?

Related to #205 - a test should only apply Terraform config that is being tested. We have a bug throughout our provider tests where we create the project using Terraform instead of during setup. It was convenient for me to make this change now because I needed access to an ID that was not available on the gitlab_branch_protection resource. Now the only Terraform config being applied in the TestAccGitLabProjectApprovalRule_basic test is the gitlab_project_approval_rule resource, and the rest is created during setup. Cleanup is also handled.

[mattkasa]

@armsnyder This looks really good, I really like how much cleaner this made the test! 💯

I'm seeing this failure locally though:

➜ make testacc-up SERVICE=gitlab-ee                                        
docker-compose up -d gitlab-ee
Docker Compose is now in the Docker CLI, try `docker compose up`

Creating network "terraform-provider-gitlab_default" with the default driver
Creating terraform-provider-gitlab_gitlab-ee_1 ... done
./scripts/await-healthy.sh
Waiting for GitLab container to become healthy...................................
GitLab is healthy
{"version":"14.0.5-ee","revision":"b044f06e4dd"}
➜ make testacc SERVICE=gitlab-ee RUN=TestAccGitLabProjectApprovalRule_basic
TF_ACC=1 GITLAB_TOKEN=ACCTEST GITLAB_BASE_URL=http://127.0.0.1:8080/api/v4 go test -v ./gitlab -test.run TestAccGitLabProjectApprovalRule_basic -timeout 40m
=== RUN   TestAccGitLabProjectApprovalRule_basic
    testing.go:705: Step 0 error: Check failed: Check 2/2 error: protected_branches
        Expected
            <[]int | len:0, cap:0>: nil
        to consist of
            <[]int | len:1, cap:1>: [18]
        the missing elements were
            <[]int | len:1, cap:1>: [18]
--- FAIL: TestAccGitLabProjectApprovalRule_basic (6.30s)
FAIL
FAIL	github.com/gitlabhq/terraform-provider-gitlab/gitlab	6.810s
FAIL
make: *** [testacc] Error 1
➜ 

I wonder why it isn't happening in CI 🤔

[armsnyder]

@mattkasa Thanks for taking a look at all that test refactoring. So the test failure makes it seem like it created the approval rule without the expected protected branch ID. If it's failing reliably can you run it again with TF_LOG=DEBUG and put the output in a gist?

[mattkasa]

@armsnyder thanks for doing all that refactoring!

Here's a link to the debug output: https://gist.github.com/mattkasa/8c99e1170f8eb3da554cd6aa74b481a3

[armsnyder]

@mattkasa In the gist I see the proetcted_branch_ids in the API request, but then it's empty in the response body. Can you check your license? Protected branches is a premium feature, so maybe there's something funky with that.

➜  curl -s -H 'PRIVATE-TOKEN: ACCTEST' http://localhost:8080/api/v4/license | jq '{plan,expired}'
{
  "plan": "premium",
  "expired": false
}

[mattkasa]

Ah you were right, my license didn't have enough users available on it for the acceptance tests, adding a different license fixed it 👍

[armsnyder]

So it wasn't related to the license tier? I was wondering whether we needed to improve test skips to be based on tier and not distribution. Anyway thanks for approving!

[mattkasa]

So it wasn't related to the license tier? I was wondering whether we needed to improve test skips to be based on tier and not distribution. Anyway thanks for approving!

Nope, my license file was just silently failing to apply 🤦