Skip to content

Commit 1268edb

Browse files
deigadeiga
authored
[MAINT] Improve provider tests (#3061)
* Ensure that `token` is never configured together with `app_auth` Signed-off-by: Timo Sand <[email protected]> * Add test to verify that `GITHUB_TOKEN` and `app_auth` are not set at the same time Signed-off-by: Timo Sand <[email protected]> * Ensure `max_per_page` test actually runs Signed-off-by: Timo Sand <[email protected]> * Ensure `max_retries` is actually tested Signed-off-by: Timo Sand <[email protected]> * Fix indentation Signed-off-by: Timo Sand <[email protected]> * Use `ExactlyOneOf` instead of `ConflictsWith` Signed-off-by: Timo Sand <[email protected]> * Add some more infor logging Signed-off-by: Timo Sand <[email protected]> * Fix brackets Signed-off-by: Timo Sand <[email protected]> * Update Org test to ensure it actually tests Signed-off-by: Timo Sand <[email protected]> --------- Signed-off-by: Timo Sand <[email protected]>
1 parent e3a3c6c commit 1268edb

2 files changed

Lines changed: 101 additions & 34 deletions

File tree

‎github/provider.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ func Provider() *schema.Provider {
1818
p := &schema.Provider{
1919
Schema: map[string]*schema.Schema{
2020
"token": {
21-
Type: schema.TypeString,
22-
Optional: true,
23-
DefaultFunc: schema.EnvDefaultFunc("GITHUB_TOKEN", nil),
24-
Description: descriptions["token"],
21+
Type: schema.TypeString,
22+
Optional: true,
23+
DefaultFunc: schema.EnvDefaultFunc("GITHUB_TOKEN", nil),
24+
Description: descriptions["token"],
25+
ExactlyOneOf: []string{"app_auth"},
2526
},
2627
"owner": {
2728
Type: schema.TypeString,
@@ -93,10 +94,11 @@ func Provider() *schema.Provider {
9394
Description: descriptions["parallel_requests"],
9495
},
9596
"app_auth": {
96-
Type: schema.TypeList,
97-
Optional: true,
98-
MaxItems: 1,
99-
Description: descriptions["app_auth"],
97+
Type: schema.TypeList,
98+
Optional: true,
99+
MaxItems: 1,
100+
Description: descriptions["app_auth"],
101+
ExactlyOneOf: []string{"token"},
100102
Elem: &schema.Resource{
101103
Schema: map[string]*schema.Schema{
102104
"id": {
@@ -414,6 +416,7 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc {
414416
}
415417

416418
if token == "" {
419+
log.Printf("[INFO] No token found, using GitHub CLI to get token from hostname %s", baseURL.Host)
417420
token = tokenFromGHCLI(baseURL)
418421
}
419422

‎github/provider_test.go‎

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"fmt"
5+
"regexp"
56
"testing"
67

78
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -53,7 +54,7 @@ func TestAccProviderConfigure(t *testing.T) {
5354
t.Run("can be configured to run anonymously", func(t *testing.T) {
5455
config := `
5556
provider "github" {
56-
token = ""
57+
token = ""
5758
}
5859
data "github_ip_ranges" "test" {}
5960
`
@@ -77,7 +78,7 @@ func TestAccProviderConfigure(t *testing.T) {
7778
insecure = true
7879
}
7980
data "github_ip_ranges" "test" {}
80-
`
81+
`
8182

8283
resource.Test(t, resource.TestCase{
8384
ProviderFactories: providerFactories,
@@ -116,39 +117,42 @@ func TestAccProviderConfigure(t *testing.T) {
116117
config := fmt.Sprintf(`
117118
provider "github" {
118119
token = "%s"
119-
owner = "%s"
120-
}`, testAccConf.token, testAccConf.owner)
120+
owner = "%[2]s"
121+
}
122+
123+
data "github_organization" "test" {
124+
name = "%[2]s"
125+
}
126+
`, testAccConf.token, testAccConf.owner)
121127

122128
resource.Test(t, resource.TestCase{
123129
PreCheck: func() { skipUnlessHasOrgs(t) },
124130
ProviderFactories: providerFactories,
125131
Steps: []resource.TestStep{
126132
{
127-
Config: config,
128-
PlanOnly: true,
129-
ExpectNonEmptyPlan: false,
133+
Config: config,
130134
},
131135
},
132136
})
137+
})
133138

134-
t.Run("can be configured with an organization account legacy", func(t *testing.T) {
135-
config := fmt.Sprintf(`
139+
t.Run("can be configured with an organization account legacy", func(t *testing.T) {
140+
config := fmt.Sprintf(`
136141
provider "github" {
137142
token = "%s"
138143
organization = "%s"
139144
}`, testAccConf.token, testAccConf.owner)
140145

141-
resource.Test(t, resource.TestCase{
142-
PreCheck: func() { skipUnlessHasOrgs(t) },
143-
ProviderFactories: providerFactories,
144-
Steps: []resource.TestStep{
145-
{
146-
Config: config,
147-
PlanOnly: true,
148-
ExpectNonEmptyPlan: false,
149-
},
146+
resource.Test(t, resource.TestCase{
147+
PreCheck: func() { skipUnlessHasOrgs(t) },
148+
ProviderFactories: providerFactories,
149+
Steps: []resource.TestStep{
150+
{
151+
Config: config,
152+
PlanOnly: true,
153+
ExpectNonEmptyPlan: false,
150154
},
151-
})
155+
},
152156
})
153157
})
154158

@@ -172,11 +176,15 @@ func TestAccProviderConfigure(t *testing.T) {
172176
})
173177

174178
t.Run("can be configured with max retries", func(t *testing.T) {
179+
testMaxRetries := -1
175180
config := fmt.Sprintf(`
176181
provider "github" {
177182
owner = "%s"
178-
max_retries = 3
179-
}`, testAccConf.owner)
183+
max_retries = %d
184+
}
185+
186+
data "github_ip_ranges" "test" {}
187+
`, testAccConf.owner, testMaxRetries)
180188

181189
resource.Test(t, resource.TestCase{
182190
PreCheck: func() { skipUnauthenticated(t) },
@@ -185,17 +193,22 @@ func TestAccProviderConfigure(t *testing.T) {
185193
{
186194
Config: config,
187195
ExpectNonEmptyPlan: false,
196+
ExpectError: regexp.MustCompile("max_retries must be greater than or equal to 0"),
188197
},
189198
},
190199
})
191200
})
192201

193202
t.Run("can be configured with max per page", func(t *testing.T) {
194-
config := `
203+
testMaxPerPage := 101
204+
config := fmt.Sprintf(`
195205
provider "github" {
196206
owner = "%s"
197-
max_per_page = 100
198-
}`
207+
max_per_page = %d
208+
}
209+
210+
data "github_ip_ranges" "test" {}
211+
`, testAccConf.owner, testMaxPerPage)
199212

200213
resource.Test(t, resource.TestCase{
201214
PreCheck: func() { skipUnauthenticated(t) },
@@ -205,13 +218,64 @@ func TestAccProviderConfigure(t *testing.T) {
205218
Config: config,
206219
ExpectNonEmptyPlan: false,
207220
Check: func(_ *terraform.State) error {
208-
if maxPerPage != 100 {
209-
return fmt.Errorf("max_per_page should be set to 100, got %d", maxPerPage)
221+
if maxPerPage != testMaxPerPage {
222+
return fmt.Errorf("max_per_page should be set to %d, got %d", testMaxPerPage, maxPerPage)
210223
}
211224
return nil
212225
},
213226
},
214227
},
215228
})
216229
})
230+
t.Run("should not allow both token and app_auth to be configured", func(t *testing.T) {
231+
config := fmt.Sprintf(`
232+
provider "github" {
233+
owner = "%s"
234+
token = "%s"
235+
app_auth {
236+
id = "1234567890"
237+
installation_id = "1234567890"
238+
pem_file = "1234567890"
239+
}
240+
}
241+
242+
data "github_ip_ranges" "test" {}
243+
`, testAccConf.owner, testAccConf.token)
244+
245+
resource.Test(t, resource.TestCase{
246+
PreCheck: func() { skipUnauthenticated(t) },
247+
ProviderFactories: providerFactories,
248+
Steps: []resource.TestStep{
249+
{
250+
Config: config,
251+
ExpectError: regexp.MustCompile("only one of `app_auth,token` can be specified"),
252+
},
253+
},
254+
})
255+
})
256+
t.Run("should not allow app_auth and GITHUB_TOKEN to be configured", func(t *testing.T) {
257+
config := fmt.Sprintf(`
258+
provider "github" {
259+
owner = "%s"
260+
app_auth {
261+
id = "1234567890"
262+
installation_id = "1234567890"
263+
pem_file = "1234567890"
264+
}
265+
}
266+
267+
data "github_ip_ranges" "test" {}
268+
`, testAccConf.owner)
269+
270+
resource.Test(t, resource.TestCase{
271+
PreCheck: func() { skipUnauthenticated(t); t.Setenv("GITHUB_TOKEN", "1234567890") },
272+
ProviderFactories: providerFactories,
273+
Steps: []resource.TestStep{
274+
{
275+
Config: config,
276+
ExpectError: regexp.MustCompile("only one of `app_auth,token` can be specified"),
277+
},
278+
},
279+
})
280+
})
217281
}

0 commit comments

Comments
 (0)