Skip to content

Commit 46162f6

Browse files
deigadeiga
committed
fixup! fixup! fixup! fixup! Add migration of the repository_id field.
This required changing all tests to need a mock of the GH API Signed-off-by: Timo Sand <[email protected]>
1 parent 9b9f8b8 commit 46162f6

2 files changed

Lines changed: 99 additions & 56 deletions

File tree

‎github/resource_github_repository_file_migration.go‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,22 +89,24 @@ func resourceGithubRepositoryFileStateUpgradeV0(ctx context.Context, rawState ma
8989
"rawState": rawState,
9090
})
9191

92-
// If branch is missing or empty, fetch the default branch from the repository
93-
if branch, ok := rawState["branch"].(string); !ok || branch == "" {
94-
meta := m.(*Owner)
95-
client := meta.v3client
96-
owner := meta.name
92+
meta := m.(*Owner)
93+
client := meta.v3client
94+
owner := meta.name
95+
96+
repoName, ok := rawState["repository"].(string)
97+
if !ok {
98+
return nil, fmt.Errorf("repository not found or is not a string")
99+
}
97100

98-
repoName, ok := rawState["repository"].(string)
99-
if !ok {
100-
return nil, fmt.Errorf("repository not found or is not a string")
101-
}
101+
repo, _, err := client.Repositories.Get(ctx, owner, repoName)
102+
if err != nil {
103+
return nil, fmt.Errorf("failed to retrieve repository '%s': %w", repoName, err)
104+
}
102105

103-
repo, _, err := client.Repositories.Get(ctx, owner, repoName)
104-
if err != nil {
105-
return nil, fmt.Errorf("failed to retrieve repository %s: %w", repoName, err)
106-
}
106+
rawState["repository_id"] = int(repo.GetID())
107107

108+
// If branch is missing or empty, fetch the default branch from the repository
109+
if branch, ok := rawState["branch"].(string); !ok || branch == "" {
108110
rawState["branch"] = repo.GetDefaultBranch()
109111
}
110112

‎github/resource_github_repository_file_migration_test.go‎

Lines changed: 84 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,36 @@ package github
22

33
import (
44
"context"
5+
"encoding/json"
6+
"fmt"
7+
"net/http"
8+
"net/url"
59
"testing"
610

711
"github.com/google/go-cmp/cmp"
12+
"github.com/google/go-github/v82/github"
813
)
914

10-
func Test_resourceGithubRepositoryFileStateUpgradeV0(t *testing.T) {
15+
func buildMockResponsesForRepositoryFileMigrationV0toV1(mockOwner, mockRepo string, wantRepoID int) []*mockResponse {
16+
responseBodyJson, err := json.Marshal(github.Repository{
17+
ID: github.Ptr(int64(wantRepoID)),
18+
DefaultBranch: github.Ptr("main"),
19+
Name: github.Ptr(mockRepo),
20+
})
21+
if err != nil {
22+
panic(fmt.Sprintf("error marshalling repository response: %s", err))
23+
}
24+
return []*mockResponse{{
25+
ExpectedUri: fmt.Sprintf("/repos/%s/%s", mockOwner, mockRepo),
26+
ExpectedHeaders: map[string]string{
27+
"Accept": "application/vnd.github.scarlet-witch-preview+json, application/vnd.github.mercy-preview+json, application/vnd.github.baptiste-preview+json, application/vnd.github.nebula-preview+json",
28+
},
29+
ResponseBody: string(responseBodyJson),
30+
StatusCode: http.StatusOK,
31+
}}
32+
}
33+
34+
func Test_resourceGithubRepositoryFileStateUpgradeV0toV1(t *testing.T) {
1135
t.Parallel()
1236

1337
for _, d := range []struct {
@@ -31,6 +55,7 @@ func Test_resourceGithubRepositoryFileStateUpgradeV0(t *testing.T) {
3155
want: map[string]any{
3256
"id": "test-repo:path/to/file.txt:main",
3357
"repository": "test-repo",
58+
"repository_id": 1234567890,
3459
"file": "path/to/file.txt",
3560
"content": "file content",
3661
"branch": "main",
@@ -50,56 +75,72 @@ func Test_resourceGithubRepositoryFileStateUpgradeV0(t *testing.T) {
5075
"branch": "develop",
5176
},
5277
want: map[string]any{
53-
"id": "test-repo:README.md:develop",
78+
"id": "test-repo:README.md:develop",
79+
"repository": "test-repo",
80+
"repository_id": 1234567890,
81+
"file": "README.md",
82+
"content": "# README",
83+
"branch": "develop",
84+
},
85+
shouldError: false,
86+
},
87+
{
88+
testName: "migrates_with_missing_branch",
89+
rawState: map[string]any{
90+
"id": "test-repo/path/to/file.txt",
5491
"repository": "test-repo",
55-
"file": "README.md",
56-
"content": "# README",
57-
"branch": "develop",
92+
"file": "path/to/file.txt",
93+
"content": "file content",
94+
},
95+
want: map[string]any{
96+
"id": "test-repo:path/to/file.txt:main",
97+
"repository": "test-repo",
98+
"repository_id": 1234567890,
99+
"file": "path/to/file.txt",
100+
"content": "file content",
101+
"branch": "main", // fetched from API
102+
},
103+
shouldError: false,
104+
},
105+
{
106+
testName: "migrates_with_empty_branch",
107+
rawState: map[string]any{
108+
"id": "test-repo/path/to/file.txt",
109+
"repository": "test-repo",
110+
"file": "path/to/file.txt",
111+
"content": "file content",
112+
"branch": "",
113+
},
114+
want: map[string]any{
115+
"id": "test-repo:path/to/file.txt:main",
116+
"repository": "test-repo",
117+
"repository_id": 1234567890,
118+
"file": "path/to/file.txt",
119+
"content": "file content",
120+
"branch": "main", // fetched from API
58121
},
59122
shouldError: false,
60123
},
61-
// TODO: Enable this test once we have a pattern to create a mock client for the test.
62-
// {
63-
// testName: "migrates_with_missing_branch",
64-
// rawState: map[string]any{
65-
// "id": "test-repo/path/to/file.txt",
66-
// "repository": "test-repo",
67-
// "file": "path/to/file.txt",
68-
// "content": "file content",
69-
// },
70-
// want: map[string]any{
71-
// "id": "test-repo:path/to/file.txt:main",
72-
// "repository": "test-repo",
73-
// "file": "path/to/file.txt",
74-
// "content": "file content",
75-
// "branch": "main", // fetched from API
76-
// },
77-
// shouldError: false,
78-
// },
79-
// TODO: Enable this test once we have a pattern to create a mock client for the test.
80-
// {
81-
// testName: "migrates_with_empty_branch",
82-
// rawState: map[string]any{
83-
// "id": "test-repo/path/to/file.txt",
84-
// "repository": "test-repo",
85-
// "file": "path/to/file.txt",
86-
// "content": "file content",
87-
// "branch": "",
88-
// },
89-
// want: map[string]any{
90-
// "id": "test-repo:path/to/file.txt:main",
91-
// "repository": "test-repo",
92-
// "file": "path/to/file.txt",
93-
// "content": "file content",
94-
// "branch": "main", // fetched from API
95-
// },
96-
// shouldError: false,
97-
// },
98124
} {
99125
t.Run(d.testName, func(t *testing.T) {
100126
t.Parallel()
101127

102-
got, err := resourceGithubRepositoryFileStateUpgradeV0(context.Background(), d.rawState, nil)
128+
meta := &Owner{
129+
name: "test-org",
130+
}
131+
132+
ts := githubApiMock(buildMockResponsesForRepositoryFileMigrationV0toV1(meta.name, d.want["repository"].(string), d.want["repository_id"].(int)))
133+
defer ts.Close()
134+
135+
httpCl := http.DefaultClient
136+
httpCl.Transport = http.DefaultTransport
137+
138+
client := github.NewClient(httpCl)
139+
u, _ := url.Parse(ts.URL + "/")
140+
client.BaseURL = u
141+
meta.v3client = client
142+
143+
got, err := resourceGithubRepositoryFileStateUpgradeV0(context.Background(), d.rawState, meta)
103144
if (err != nil) != d.shouldError {
104145
t.Fatalf("unexpected error state: got error %v, shouldError %v", err, d.shouldError)
105146
}

0 commit comments

Comments
 (0)