[BUG] fix rulesets update_allows_fetch_and_merge validation#3547
Conversation
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
…n non-forked repos Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
e68c40e to
ce2ffca
Compare
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
| resource "github_repository_ruleset" "test" { | ||
| name = "%s" | ||
| repository = github_repository.test.name | ||
| name = "%[3]s" | ||
| repository = "%[2]s" |
| } | ||
| } | ||
|
|
||
| func mustArchiveTestRepository(t *testing.T, repo *github.Repository) { |
There was a problem hiding this comment.
If you need to retry in this function, which I question for a test function, then could you use retryUntilOK. That said see below for what I think might be the actual solution.
There was a problem hiding this comment.
I'll look at retry until ok.
Retry is my choice as a static sleep felt worse than a retry. And since repo creation is async and archiving right after creation usually returns a 422, something is necessary
There was a problem hiding this comment.
Given that it's for a test, wouldn't the flexible repo create pattern below be a better option as it's allow you to create a repo already archived?
There was a problem hiding this comment.
It would be, but it's not possible to create repos as "archived" 🫠
There was a problem hiding this comment.
I did wonder, so I think using retryUntilOK in the helper would be the best bet.
| return mustCreateConfigurableTestRepository(t, "internal") | ||
| } | ||
|
|
||
| func mustCreateConfigurableTestRepository(t *testing.T, visibility string) *github.Repository { |
There was a problem hiding this comment.
The pattern I'm working towards is below, with the expectation that we probably want to move them into separate files such as acc_helpers_repository_test.go.
type createTestRepositoryOptionsFunc func(*github.Repository)
func mustCreateTestRepository(t *testing.T, f ...createTestRepositoryOptionsFunc) *github.Repository {
t.Helper()
randomID := acctest.RandString(testRandomIDLength)
name := fmt.Sprintf("%s%s", testResourcePrefix, randomID)
req := &github.Repository{
Name: &name,
AutoInit: new(true),
}
for _, fn := range f {
fn(req)
}
repo, _, err := testAccConf.meta.v3client.Repositories.Create(t.Context(), testAccConf.meta.name, req)
if err != nil {
t.Fatalf("failed to create test repository: %v", err)
}
t.Cleanup(func() {
if _, err := testAccConf.meta.v3client.Repositories.Delete(context.Background(), testAccConf.meta.name, name); err != nil {
if err, ok := errors.AsType[*github.ErrorResponse](err); ok && err.Response.StatusCode == 404 {
return
}
t.Logf("failed to delete test repository %s: %v", name, err)
}
})
return repo
}There was a problem hiding this comment.
This is way better! I'll adopt this
| visibility = "%s" | ||
| } | ||
|
|
||
| resource "github_repository_environment" "example" { |
There was a problem hiding this comment.
You can create the environment outside of the TF cnfig too.
| resource.TestCheckResourceAttr("github_repository_ruleset.test", "rules.0.copilot_code_review.0.review_draft_pull_requests", "false"), | ||
| ), | ||
| ConfigStateChecks: []statecheck.StateCheck{ | ||
| statecheck.ExpectKnownValue("github_repository_ruleset.test", tfjsonpath.New("name"), knownvalue.StringExact("test")), |
There was a problem hiding this comment.
You don't need to check for value in the config, the framework does this for you.
Resolves #3543
Before the change?
rules.0.update_allows_fetch_and_mergeis being marked as invalid config for repository rulesetAfter the change?
rules.0.update_allows_fetch_and_mergewill be allowed as config for repository rulesetsrules.0.update_allows_fetch_and_mergegithub_repository_rulesettests to follow modern practicesPull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!