Skip to content

Commit c9eb620

Browse files
jai-deepsourcejai-deepsource
committed
Fix CI deploy paths, improve panic handling, and expand test coverage
- Use dynamic base URL for legacy install script instead of hardcoded deepsource.io/cli - Remove prod/dev prefix branching in R2 upload, always use empty prefix - Extract mainRun() with named return so defer handles panic exit code without calling os.Exit in deferred func - Split ResolveAutoBranch into resolveWithPR/resolveWithoutPR helpers - Handle os.Setenv/os.Unsetenv errors in report test setup - Add unit tests for detectProvider, extractRepoName, extractOwner, extractADSOwner - Add GitLab, Bitbucket, and unsupported provider cases for GetRemoteMap - Add tests for testutil helpers (LoadGoldenFile, MockQueryFunc, CreateTestConfigManager)
1 parent 734b2e5 commit c9eb620

6 files changed

Lines changed: 276 additions & 37 deletions

File tree

‎.github/workflows/build-and-deploy.yml‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ jobs:
271271
-e 's|__DEFAULT_INSTALL_DIR__|${HOME}/.local/bin|g' \
272272
scripts/install.sh.template > artifacts/install
273273
274-
# Legacy install script (deepsource.io/cli)
275-
sed -e "s|__BASE_URL__|https://deepsource.io/cli|g" \
274+
# Legacy install script (deepsource.io/cli backward compat)
275+
sed -e "s|__BASE_URL__|${{ needs.resolve-env.outputs.base_url }}|g" \
276276
-e 's|__BINARY_NAME__|deepsource|g' \
277277
-e 's|__DEFAULT_INSTALL_DIR__|./bin|g' \
278278
scripts/install.sh.template > artifacts/install-legacy
@@ -290,12 +290,7 @@ jobs:
290290
BUCKET="${{ secrets.R2_DEV_BUCKET_NAME }}"
291291
fi
292292
293-
# Prod stores under cli/ prefix; dev serves bucket root via subdomain
294-
if [ "${{ needs.resolve-env.outputs.environment }}" = "prod" ]; then
295-
PREFIX="cli/"
296-
else
297-
PREFIX=""
298-
fi
293+
PREFIX=""
299294
300295
# Upload archives and checksums (immutable)
301296
for f in artifacts/${{ needs.resolve-env.outputs.binary_name }}_*; do

‎cmd/deepsource/main.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ func sentryEnvironment(ver string) string {
3737
}
3838

3939
func main() {
40+
os.Exit(mainRun())
41+
}
42+
43+
func mainRun() (exitCode int) {
4044
log.SetFlags(log.LstdFlags | log.Lshortfile)
4145

4246
// Override app identity for dev builds
@@ -62,12 +66,11 @@ func main() {
6266
sentry.CurrentHub().Recover(r)
6367
sentry.Flush(2 * time.Second)
6468
fmt.Fprintf(os.Stderr, "fatal: unexpected panic: %v\n", r)
65-
os.Exit(2)
69+
exitCode = 2
6670
}
6771
}()
6872

69-
exitCode := run()
70-
os.Exit(exitCode)
73+
return run()
7174
}
7275

7376
func run() int {

‎command/cmdutil/resolve_run.go‎

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -242,34 +242,57 @@ func ResolveAutoBranch(
242242
// Check for an open PR on this branch.
243243
if prNumber, found := ResolvePRForBranch(ctx, client, branchName, remote); found {
244244
result.PRNumber = prNumber
245+
return resolveWithPR(ctx, w, client, branchName, remote, result)
246+
}
247+
248+
return resolveWithoutPR(ctx, w, client, branchNameFunc, branchName, remote, result)
249+
}
245250

246-
run, runErr := ResolveLatestRunForBranch(ctx, client, branchName, remote)
247-
if runErr == nil && IsRunInProgress(run.Status) {
248-
finalStatus, waitErr := WaitOrFallback(
249-
ctx, w, run.Status, run.CommitOid[:8], branchName, 5*time.Second,
250-
func(ctx context.Context) (string, error) {
251-
r, err := ResolveLatestRunForBranch(ctx, client, branchName, remote)
252-
if err != nil {
253-
return "", err
254-
}
255-
return r.Status, nil
256-
})
257-
if waitErr != nil {
258-
return nil, waitErr
259-
}
260-
if IsRunTimedOut(finalStatus) {
261-
style.Warnf(w, "Analysis timed out for branch %q.", branchName)
262-
result.Empty = true
263-
return result, nil
264-
}
251+
// resolveWithPR handles branch resolution when an open PR exists.
252+
func resolveWithPR(
253+
ctx context.Context,
254+
w io.Writer,
255+
client *deepsource.Client,
256+
branchName string,
257+
remote *vcs.RemoteData,
258+
result *AutoBranchResult,
259+
) (*AutoBranchResult, error) {
260+
run, runErr := ResolveLatestRunForBranch(ctx, client, branchName, remote)
261+
if runErr == nil && IsRunInProgress(run.Status) {
262+
finalStatus, waitErr := WaitOrFallback(
263+
ctx, w, run.Status, run.CommitOid[:8], branchName, 5*time.Second,
264+
func(ctx context.Context) (string, error) {
265+
r, err := ResolveLatestRunForBranch(ctx, client, branchName, remote)
266+
if err != nil {
267+
return "", err
268+
}
269+
return r.Status, nil
270+
})
271+
if waitErr != nil {
272+
return nil, waitErr
265273
}
266-
if runErr == nil {
267-
result.CommitOid = run.CommitOid
274+
if IsRunTimedOut(finalStatus) {
275+
style.Warnf(w, "Analysis timed out for branch %q.", branchName)
276+
result.Empty = true
277+
return result, nil
268278
}
269-
return result, nil
270279
}
280+
if runErr == nil {
281+
result.CommitOid = run.CommitOid
282+
}
283+
return result, nil
284+
}
271285

272-
// No PR — resolve latest run on the branch.
286+
// resolveWithoutPR handles branch resolution when no PR exists.
287+
func resolveWithoutPR(
288+
ctx context.Context,
289+
w io.Writer,
290+
client *deepsource.Client,
291+
branchNameFunc func() (string, error),
292+
branchName string,
293+
remote *vcs.RemoteData,
294+
result *AutoBranchResult,
295+
) (*AutoBranchResult, error) {
273296
commitOid, _, runStatus, resolveErr := ResolveLatestRun(ctx, client, branchNameFunc, remote)
274297
if resolveErr != nil {
275298
if branchName != "" && branchName == GetDefaultBranch() {

‎command/report/tests/init_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,13 @@ func TestMain(m *testing.M) {
3636
}
3737
code := m.Run()
3838
if hadCodePath {
39-
os.Setenv("CODE_PATH", origCodePath)
39+
if err := os.Setenv("CODE_PATH", origCodePath); err != nil {
40+
log.Printf("failed to restore CODE_PATH: %v", err)
41+
}
4042
} else {
41-
os.Unsetenv("CODE_PATH")
43+
if err := os.Unsetenv("CODE_PATH"); err != nil {
44+
log.Printf("failed to unset CODE_PATH: %v", err)
45+
}
4246
}
4347
srv.Close()
4448
os.Exit(code)
@@ -81,7 +85,9 @@ func prepareArtifacts() error {
8185
}
8286
}
8387
repoRoot = rootDir
84-
_ = os.Setenv("CODE_PATH", repoRoot)
88+
if err := os.Setenv("CODE_PATH", repoRoot); err != nil {
89+
return fmt.Errorf("failed to set CODE_PATH: %w", err)
90+
}
8591

8692
tempDir, err := os.MkdirTemp("", "deepsource-report")
8793
if err != nil {

‎internal/testutil/testutil_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package testutil
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
func TestLoadGoldenFile(t *testing.T) {
10+
// Create a temp file with known content.
11+
dir := t.TempDir()
12+
path := filepath.Join(dir, "test.json")
13+
content := []byte(`{"key": "value"}`)
14+
if err := os.WriteFile(path, content, 0o644); err != nil {
15+
t.Fatalf("failed to write temp file: %v", err)
16+
}
17+
18+
got := LoadGoldenFile(t, path)
19+
if string(got) != string(content) {
20+
t.Errorf("LoadGoldenFile returned %q, want %q", got, content)
21+
}
22+
}
23+
24+
func TestMockQueryFunc(t *testing.T) {
25+
// Create a golden file with a JSON object.
26+
dir := t.TempDir()
27+
path := filepath.Join(dir, "response.json")
28+
if err := os.WriteFile(path, []byte(`{"name":"test"}`), 0o644); err != nil {
29+
t.Fatalf("failed to write golden file: %v", err)
30+
}
31+
32+
mock := MockQueryFunc(t, map[string]string{
33+
"GetIssues": path,
34+
})
35+
36+
var result struct{ Name string }
37+
err := mock.QueryFunc(nil, "query GetIssues { ... }", nil, &result)
38+
if err != nil {
39+
t.Fatalf("QueryFunc returned error: %v", err)
40+
}
41+
if result.Name != "test" {
42+
t.Errorf("got Name=%q, want %q", result.Name, "test")
43+
}
44+
}
45+
46+
func TestCreateTestConfigManager(t *testing.T) {
47+
mgr := CreateTestConfigManager(t, "test-token", "https://app.deepsource.com", "testuser")
48+
cfg, err := mgr.Load()
49+
if err != nil {
50+
t.Fatalf("failed to read config: %v", err)
51+
}
52+
if cfg.Token != "test-token" {
53+
t.Errorf("got Token=%q, want %q", cfg.Token, "test-token")
54+
}
55+
if cfg.Host != "https://app.deepsource.com" {
56+
t.Errorf("got Host=%q, want %q", cfg.Host, "https://app.deepsource.com")
57+
}
58+
if cfg.User != "testuser" {
59+
t.Errorf("got User=%q, want %q", cfg.User, "testuser")
60+
}
61+
if cfg.IsExpired() {
62+
t.Error("expected token to not be expired")
63+
}
64+
}
65+
66+
func TestCreateExpiredTestConfigManager(t *testing.T) {
67+
mgr := CreateExpiredTestConfigManager(t, "expired-token", "https://app.deepsource.com", "testuser")
68+
cfg, err := mgr.Load()
69+
if err != nil {
70+
t.Fatalf("failed to read config: %v", err)
71+
}
72+
if cfg.Token != "expired-token" {
73+
t.Errorf("got Token=%q, want %q", cfg.Token, "expired-token")
74+
}
75+
if !cfg.IsExpired() {
76+
t.Error("expected token to be expired")
77+
}
78+
}

‎internal/vcs/remotes_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,108 @@ var adsLegacySSHRemoteMap = map[string][]string{
5656
}
5757
var adsLegacySSHRemoteList = []string{"origin myorg@vs-ssh.visualstudio.com:v3/myorg/myproject/myrepo (fetch)", "origin myorg@vs-ssh.visualstudio.com:v3/myorg/myproject/myrepo (push)"}
5858

59+
func TestDetectProvider(t *testing.T) {
60+
tests := []struct {
61+
url string
62+
want string
63+
}{
64+
{"git@github.com:user/repo.git", "GITHUB"},
65+
{"https://github.com/user/repo", "GITHUB"},
66+
{"git@gitlab.com:user/repo.git", "GITLAB"},
67+
{"https://gitlab.com/user/repo", "GITLAB"},
68+
{"git@bitbucket.org:user/repo.git", "BITBUCKET"},
69+
{"https://bitbucket.org/user/repo", "BITBUCKET"},
70+
{"https://dev.azure.com/org/proj/_git/repo", "ADS"},
71+
{"https://org.visualstudio.com/proj/_git/repo", "ADS"},
72+
{"git@ssh.dev.azure.com:v3/org/proj/repo", "ADS"},
73+
{"https://example.com/user/repo", ""},
74+
{"git@selfhosted.example.com:user/repo.git", ""},
75+
}
76+
for _, tt := range tests {
77+
t.Run(tt.url, func(t *testing.T) {
78+
got := detectProvider(tt.url)
79+
if got != tt.want {
80+
t.Errorf("detectProvider(%q) = %q, want %q", tt.url, got, tt.want)
81+
}
82+
})
83+
}
84+
}
85+
86+
func TestExtractRepoName(t *testing.T) {
87+
tests := []struct {
88+
url string
89+
want string
90+
}{
91+
{"git@github.com:user/myrepo.git", "myrepo"},
92+
{"https://github.com/user/myrepo", "myrepo"},
93+
{"https://github.com/user/myrepo.git", "myrepo"},
94+
{"https://dev.azure.com/org/proj/_git/myrepo", "myrepo"},
95+
{"git@ssh.dev.azure.com:v3/org/proj/myrepo", "myrepo"},
96+
}
97+
for _, tt := range tests {
98+
t.Run(tt.url, func(t *testing.T) {
99+
got := extractRepoName(tt.url)
100+
if got != tt.want {
101+
t.Errorf("extractRepoName(%q) = %q, want %q", tt.url, got, tt.want)
102+
}
103+
})
104+
}
105+
}
106+
107+
func TestExtractOwner(t *testing.T) {
108+
tests := []struct {
109+
url string
110+
provider string
111+
want string
112+
}{
113+
// SSH GitHub
114+
{"git@github.com:myowner/repo.git", "GITHUB", "myowner"},
115+
// HTTPS GitHub
116+
{"https://github.com/myowner/repo", "GITHUB", "myowner"},
117+
// HTTPS with PAT
118+
{"https://user:token@github.com/myowner/repo", "GITHUB", "myowner"},
119+
// SSH GitLab
120+
{"git@gitlab.com:myowner/repo.git", "GITLAB", "myowner"},
121+
// HTTPS GitLab
122+
{"https://gitlab.com/myowner/repo", "GITLAB", "myowner"},
123+
// ADS delegation
124+
{"https://dev.azure.com/myorg/proj/_git/repo", "ADS", "myorg"},
125+
// Unknown prefix
126+
{"ftp://example.com/owner/repo", "GITHUB", ""},
127+
}
128+
for _, tt := range tests {
129+
t.Run(tt.url, func(t *testing.T) {
130+
got := extractOwner(tt.url, tt.provider)
131+
if got != tt.want {
132+
t.Errorf("extractOwner(%q, %q) = %q, want %q", tt.url, tt.provider, got, tt.want)
133+
}
134+
})
135+
}
136+
}
137+
138+
func TestExtractADSOwner(t *testing.T) {
139+
tests := []struct {
140+
name string
141+
url string
142+
want string
143+
}{
144+
{"HTTPS dev.azure.com", "https://dev.azure.com/myorg/proj/_git/repo", "myorg"},
145+
{"HTTPS visualstudio.com", "https://myorg.visualstudio.com/proj/_git/repo", "myorg"},
146+
{"SSH dev.azure.com", "git@ssh.dev.azure.com:v3/myorg/proj/repo", "myorg"},
147+
{"SSH visualstudio.com", "myorg@vs-ssh.visualstudio.com:v3/myorg/proj/repo", "myorg"},
148+
{"HTTPS dev.azure.com empty path", "https://dev.azure.com/", ""},
149+
{"HTTPS invalid URL", "https://://bad", ""},
150+
}
151+
for _, tt := range tests {
152+
t.Run(tt.name, func(t *testing.T) {
153+
got := extractADSOwner(tt.url)
154+
if got != tt.want {
155+
t.Errorf("extractADSOwner(%q) = %q, want %q", tt.url, got, tt.want)
156+
}
157+
})
158+
}
159+
}
160+
59161
// TestGetRemoteMap_EmptyInput verifies that an empty remote list returns an empty map.
60162
// Note: the ListRemotes() code path (auto-detecting remotes from git) requires an
61163
// integration test environment with a real git process, so it is not covered here.
@@ -116,6 +218,34 @@ func TestGetRemoteMap(t *testing.T) {
116218
adsLegacySSHRemoteList,
117219
adsLegacySSHRemoteMap,
118220
},
221+
{
222+
"GitLab HTTPS URLs",
223+
[]string{
224+
"origin https://gitlab.com/myuser/myrepo (fetch)",
225+
"origin https://gitlab.com/myuser/myrepo (push)",
226+
},
227+
map[string][]string{
228+
"origin": {"myuser", "myrepo", "GITLAB", "myuser/myrepo"},
229+
},
230+
},
231+
{
232+
"Bitbucket SSH URLs",
233+
[]string{
234+
"origin git@bitbucket.org:myuser/myrepo.git (fetch)",
235+
"origin git@bitbucket.org:myuser/myrepo.git (push)",
236+
},
237+
map[string][]string{
238+
"origin": {"myuser", "myrepo", "BITBUCKET", "myuser/myrepo"},
239+
},
240+
},
241+
{
242+
"unsupported provider is skipped",
243+
[]string{
244+
"origin https://selfhosted.example.com/user/repo (fetch)",
245+
"origin https://selfhosted.example.com/user/repo (push)",
246+
},
247+
map[string][]string{},
248+
},
119249
}
120250

121251
for _, tt := range tests {
@@ -126,6 +256,10 @@ func TestGetRemoteMap(t *testing.T) {
126256
return
127257
}
128258

259+
if len(remoteMap) != len(tt.want) {
260+
t.Errorf("got %d remotes, want %d", len(remoteMap), len(tt.want))
261+
}
262+
129263
for remote := range remoteMap {
130264
got := remoteMap[remote]
131265
want := tt.want[remote]

0 commit comments

Comments
 (0)