Skip to content

Commit b1f33ea

Browse files
SuperCoolPencilMeet Mehta
andauthored
Add categories (#209)
* chore: rm unused files * use github.com/adrg/xdg for dir support remove migration * add category.go * chore: formatting * refactor: replace custom implementations with open-source libraries - Use github.com/dustin/go-humanize for size formatting - Use github.com/skratchdot/open-golang/open for opening browsers/files - Use github.com/kennygrant/sanitize for filename sanitization - Update tests to reflect new library behaviors - Fix remaining ConvertBytesToHumanReadable usage causing build errors * fix tests * add fallbacks to paths * fix flaky test * remove redundant Megabyte * effort to remove magic numbers * yet another attempt to fix flaky test * blablabla * remove test * fix ci * update build.yml * update lint.yml * feat(config): add categories type and default definitions * replace category with categories.go * feat(config): implement category matching and path resolution with tests * feat(config): integrate categories into general settings * feat(tui): auto route downloads based on categories * fix(tui): only auto-route categories on default download paths * feat(tui): establish model states and keybindings for category manager * feat(tui): implement split pane UI for category manager * feat(tui): implement edit mode and regex configuration for categories * feat(tui): connect settings menu to category manager * fix(tui): restore category routing and filtering flows * fix(config): honor runtime XDG path overrides * ci: detect go coverprofile support correctly * fix(config): preserve explicit empty categories in settings * fix(startup): migrate legacy state db into state dir * fix(utils): fallback when sanitization yields extension-only name * fix(config): default category paths fallback to downloads * ci: pin lint tool and scope coverage profile to Linux * test(cmd): isolate XDG state and enforce strict JSON assertions * fix(startup): migrate state only after single-instance lock * fix(config): harden category/path handling and unify size constants * fix(tui): validate category edits and harden routing edge cases * ci: use supported setup-go version selector * fix(cmd): tighten file migration close handling and test env setup * fix(config): harden runtime/category edge cases and metric underflow * fix(tui): surface category match errors and route filename uniqueness by final path * ci: only clear stale failure comment after successful matrix * fix(cmd): harden migration sidecar handling and file mode copy * fix(config): guard nil categories and normalize fallback paths * fix(tui): preserve request ids when queuing local downloads * ci: fail when test job fails without parsed failing artifacts * fix(cmd): abort on migration errors and serialize XDG test isolation * fix(config): validate category regex and only use existing default dirs * fix(core): stop report loop on shutdown and check persisted ID collisions * fix(pool): guard progressCh sends to prevent panic on closed channel Worker goroutines in WorkerPool outlive GracefulShutdown because taskChan was never closed. If a worker picks up a late task and errors, it sends on progressCh which may already be closed by LocalDownloadService.Shutdown, causing panic: send on closed channel. Add a progressDone channel that is closed at the end of GracefulShutdown. A new trySendProgress helper selects on progressDone before sending, silently dropping the message if shutdown is in progress. Also close taskChan after wg.Wait so worker goroutines exit their range loop. Fixes flaky CI failures where all tests pass individually but go test exits non-zero due to an unrecovered goroutine panic. * fix: apply review findings — pin Go version, surface GetDownload errors, reorder lock acquisition 1. CI: pin test job to go-version 1.24.4 with check-latest false, matching the release job's toolchain. 2. local_service.go: propagate state.GetDownload errors in the duplicate-ID check instead of silently treating DB failures as uniqueness passes. state.go: GetDownload returns nil,nil when the DB is uninitialized (consistent with LoadMasterList). 3. server.go: move AcquireLock() before mustInitializeGlobalState() in both serverCmd and serverStartCmd to prevent concurrent DB mutations when two processes start simultaneously. 4. http_handler_test.go: initialize state DB so the stricter GetDownload error check has a working database. * fix(download): guard ProgressCh sends in TUIDownload against closed channel TUIDownload sends DownloadStartedMsg and DownloadCompleteMsg directly on cfg.ProgressCh, bypassing the pool's trySendProgress guard. During shutdown, the channel is closed while a worker goroutine may still be inside TUIDownload, causing panic: send on closed channel. Add safeSendProgress helper that uses defer/recover to catch panics from sending on a closed channel. Apply it to both direct sends in manager.go. * chore: go mod tidy — promote go-humanize to direct dependency * chore: go fmt * chore: fix ci * fix lint errors * dont reinvent the wheel * chore: update lint version * Goroutine leak fixes * chore: fix typo * chore: update go version to 1.25.0 * qol --------- Co-authored-by: Meet Mehta <supercoolpencil@duck.com> Co-authored-by: Meet Mehta <--global>
1 parent 51c95b4 commit b1f33ea

74 files changed

Lines changed: 2395 additions & 1329 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/benchmark.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ jobs:
4545
- name: Set up Go
4646
uses: actions/setup-go@v5
4747
with:
48-
go-version: "1.24.4+auto"
48+
go-version: "1.25.0+auto"
4949
cache-dependency-path: current/go.sum
5050

5151
- name: Update APT

‎.github/workflows/build.yml‎

Lines changed: 18 additions & 214 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ on:
1414

1515
permissions:
1616
contents: write
17+
checks: write
1718

1819
jobs:
1920
test:
@@ -28,231 +29,34 @@ jobs:
2829
- name: Set up Go
2930
uses: actions/setup-go@v5
3031
with:
31-
go-version: "1.24.4+auto"
32+
go-version: "1.25.0"
33+
check-latest: false
34+
- name: Install gotestsum
35+
run: go install gotest.tools/gotestsum@latest
3236
- name: Build
3337
shell: bash
3438
run: go build -v ./...
3539
- name: Test
3640
shell: bash
37-
env:
38-
MATRIX_OS: ${{ matrix.os }}
3941
run: |
40-
set +e
41-
out_file="go-test-${MATRIX_OS}.jsonl"
42-
go test -json -coverprofile=coverage.out "./..." 2>&1 | tee "$out_file"
43-
status=${PIPESTATUS[0]}
44-
45-
failed_tests_file="failed_tests_${MATRIX_OS}.txt"
46-
failed_packages_file="failed_packages_${MATRIX_OS}.txt"
47-
exit_code_file="test_exit_code_${MATRIX_OS}.txt"
48-
summary_file="test_summary_${MATRIX_OS}.txt"
49-
test_results_file="test_results_${MATRIX_OS}.tsv"
50-
51-
awk '
52-
/"Test":"/ && /"Action":"(pass|fail)"/ {
53-
action = ""
54-
test = ""
55-
if (match($0, /"Action":"[^"]+"/)) {
56-
action = substr($0, RSTART + 10, RLENGTH - 11)
57-
}
58-
if (match($0, /"Test":"[^"]+"/)) {
59-
test = substr($0, RSTART + 8, RLENGTH - 9)
60-
}
61-
if ((action == "pass" || action == "fail") && test != "") {
62-
status[test] = action
63-
}
64-
}
65-
END {
66-
for (t in status) {
67-
print t "\t" status[t]
68-
}
69-
}
70-
' "$out_file" > "$test_results_file"
71-
72-
awk -F '\t' '$2 == "fail" { print $1 }' "$test_results_file" | sort -u > "$failed_tests_file"
73-
74-
grep -E '^FAIL[[:space:]]' "$out_file" | sed 's/\r$//' > "$failed_packages_file" || true
75-
total_tests=$(wc -l < "$test_results_file" | tr -d ' ')
76-
passed_tests=$(awk -F '\t' '$2 == "pass" { c++ } END { print c + 0 }' "$test_results_file")
77-
failed_tests=$(awk -F '\t' '$2 == "fail" { c++ } END { print c + 0 }' "$test_results_file")
78-
79-
{
80-
printf 'total=%s\n' "$total_tests"
81-
printf 'passed=%s\n' "$passed_tests"
82-
printf 'failed=%s\n' "$failed_tests"
83-
} > "$summary_file"
84-
85-
printf '%s\n' "$status" > "$exit_code_file"
86-
87-
exit "$status"
88-
- name: Upload test failure data
89-
if: always() && github.event_name == 'pull_request'
90-
uses: actions/upload-artifact@v4
42+
cover_arg=""
43+
if [ "$RUNNER_OS" = "Linux" ]; then
44+
cover_arg="-- -coverprofile=coverage.out"
45+
fi
46+
gotestsum --junitfile test-results.xml --format testdox $cover_arg ./...
47+
- name: Test Report
48+
uses: dorny/test-reporter@v1
49+
if: always()
9150
with:
92-
name: test-failure-data-${{ matrix.os }}
93-
path: |
94-
failed_tests_${{ matrix.os }}.txt
95-
failed_packages_${{ matrix.os }}.txt
96-
test_exit_code_${{ matrix.os }}.txt
97-
test_summary_${{ matrix.os }}.txt
98-
if-no-files-found: ignore
51+
name: Test Results (${{ matrix.os }})
52+
path: test-results.xml
53+
reporter: java-junit
9954
- name: Upload coverage reports to Codecov
10055
uses: codecov/codecov-action@v5
101-
if: matrix.os == 'ubuntu-latest' && success()
56+
if: matrix.os == 'ubuntu-latest' && success() && hashFiles('coverage.out') != ''
10257
with:
10358
token: ${{ secrets.CODECOV_TOKEN }}
10459

105-
comment-failed-tests:
106-
name: Comment Failed Tests
107-
needs: test
108-
if: always() && github.event_name == 'pull_request'
109-
runs-on: ubuntu-latest
110-
permissions:
111-
contents: read
112-
issues: write
113-
pull-requests: write
114-
steps:
115-
- name: Download failure artifacts
116-
continue-on-error: true
117-
uses: actions/download-artifact@v4
118-
with:
119-
path: test-failure-data
120-
pattern: test-failure-data-*
121-
merge-multiple: true
122-
- name: Upsert PR comment with failed tests
123-
uses: actions/github-script@v7
124-
env:
125-
NEEDS_TEST_RESULT: ${{ needs.test.result }}
126-
with:
127-
script: |
128-
const fs = require('fs');
129-
const path = require('path');
130-
131-
const marker = '<!-- surge-test-failures -->';
132-
const artifactDir = 'test-failure-data';
133-
134-
function readLines(filePath) {
135-
if (!fs.existsSync(filePath)) return [];
136-
return fs.readFileSync(filePath, 'utf8')
137-
.split(/\r?\n/)
138-
.map((s) => s.trim())
139-
.filter(Boolean);
140-
}
141-
142-
function escapeBackticks(s) {
143-
return s.replace(/`/g, '\\`');
144-
}
145-
146-
function readSummary(filePath) {
147-
const out = { total: 0, passed: 0, failed: 0 };
148-
for (const line of readLines(filePath)) {
149-
const [k, v] = line.split('=');
150-
const n = Number.parseInt(v || '0', 10);
151-
if (k && Number.isFinite(n)) out[k] = n;
152-
}
153-
return out;
154-
}
155-
156-
const byOS = new Map();
157-
if (fs.existsSync(artifactDir)) {
158-
for (const name of fs.readdirSync(artifactDir)) {
159-
const m = name.match(/^test_exit_code_(.+)\.txt$/);
160-
if (!m) continue;
161-
const osName = m[1];
162-
const exitCodeLines = readLines(path.join(artifactDir, `test_exit_code_${osName}.txt`));
163-
const exitCode = Number.parseInt(exitCodeLines[0] || '0', 10);
164-
const failedTests = readLines(path.join(artifactDir, `failed_tests_${osName}.txt`));
165-
const failedPackages = readLines(path.join(artifactDir, `failed_packages_${osName}.txt`));
166-
const summary = readSummary(path.join(artifactDir, `test_summary_${osName}.txt`));
167-
byOS.set(osName, { exitCode, failedTests, failedPackages, summary });
168-
}
169-
}
170-
core.info(`Parsed failure artifacts for OS entries: ${[...byOS.keys()].join(', ') || '(none)'}`);
171-
core.info(`needs.test.result=${process.env.NEEDS_TEST_RESULT || '(unset)'}`);
172-
173-
const failing = [...byOS.entries()]
174-
.filter(([, data]) => data.exitCode !== 0)
175-
.sort(([a], [b]) => a.localeCompare(b));
176-
177-
if (!context.payload.pull_request) {
178-
core.info('No pull_request payload; skipping comment upsert.');
179-
return;
180-
}
181-
182-
if (failing.length === 0 && process.env.NEEDS_TEST_RESULT && process.env.NEEDS_TEST_RESULT !== 'success' && byOS.size === 0) {
183-
core.setFailed('Tests did not succeed but no test failure artifacts were found; cannot post failure comment.');
184-
return;
185-
}
186-
187-
const issueNumber = context.payload.pull_request.number;
188-
const comments = await github.paginate(github.rest.issues.listComments, {
189-
owner: context.repo.owner,
190-
repo: context.repo.repo,
191-
issue_number: issueNumber,
192-
per_page: 100,
193-
});
194-
const existing = comments.find((c) => c.user?.type === 'Bot' && c.body?.includes(marker));
195-
196-
if (failing.length === 0) {
197-
if (existing) {
198-
try {
199-
await github.rest.issues.deleteComment({
200-
owner: context.repo.owner,
201-
repo: context.repo.repo,
202-
comment_id: existing.id,
203-
});
204-
} catch (err) {
205-
core.warning(`Failed deleting stale failure comment: ${err.message}`);
206-
}
207-
}
208-
return;
209-
}
210-
211-
const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
212-
const sections = failing.map(([osName, data]) => {
213-
const lines = [`### \`${osName}\``];
214-
lines.push(`Summary: ${data.summary.passed}/${data.summary.total} tests passed, ${data.summary.failed}/${data.summary.total} tests failed`);
215-
lines.push('');
216-
if (data.failedTests.length > 0) {
217-
lines.push('Failed tests:');
218-
for (const testName of data.failedTests) {
219-
lines.push(`- \`${escapeBackticks(testName)}\``);
220-
}
221-
} else if (data.failedPackages.length > 0) {
222-
lines.push('Go reported package/setup failure:');
223-
for (const pkg of data.failedPackages.slice(0, 20)) {
224-
lines.push(`- \`${escapeBackticks(pkg)}\``);
225-
}
226-
} else {
227-
lines.push('- `go test` failed before emitting a test-level failure line.');
228-
}
229-
return lines.join('\n');
230-
});
231-
232-
const body = [
233-
marker,
234-
'## Go Test Failures',
235-
`Run: [${context.runId}](${runUrl})`,
236-
'',
237-
...sections,
238-
].join('\n');
239-
240-
if (existing) {
241-
await github.rest.issues.updateComment({
242-
owner: context.repo.owner,
243-
repo: context.repo.repo,
244-
comment_id: existing.id,
245-
body,
246-
});
247-
} else {
248-
await github.rest.issues.createComment({
249-
owner: context.repo.owner,
250-
repo: context.repo.repo,
251-
issue_number: issueNumber,
252-
body,
253-
});
254-
}
255-
25660
release:
25761
name: Release
25862
runs-on: ubuntu-latest
@@ -266,7 +70,7 @@ jobs:
26670
- name: Set up Go
26771
uses: actions/setup-go@v5
26872
with:
269-
go-version: "1.24.4+auto"
73+
go-version: "1.25.0+auto"
27074
- name: Run GoReleaser
27175
uses: goreleaser/goreleaser-action@v6
27276
with:

‎.github/workflows/lint.yml‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ jobs:
1717
- uses: actions/checkout@v4
1818
- uses: actions/setup-go@v5
1919
with:
20-
go-version: '1.24.4+auto'
20+
go-version: '1.25.0'
2121
- name: golangci-lint
22-
uses: golangci/golangci-lint-action@v6
23-
with:
24-
version: v1.64
22+
run: go run github.com/golangci/golangci-lint/cmd/golangci-lint@latest run

‎TODO.md‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎analyze.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@
2323
except ImportError:
2424
HAS_MATPLOTLIB = False
2525

26-
MB = 1024 * 1024
27-
GB = 1024 * 1024 * 1024
26+
KB = 1 << 10
27+
MB = 1 << 20
28+
GB = 1 << 30
2829

2930

3031
@dataclass

‎benchmark.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
# =============================================================================
2626
IS_WINDOWS = platform.system() == "Windows"
2727
EXE_SUFFIX = ".exe" if IS_WINDOWS else ""
28-
MB = 1024 * 1024
28+
KB = 1 << 10
29+
MB = 1 << 20
2930
DEFAULT_TEST_URL = "https://sin-speed.hetzner.com/1GB.bin"
3031

3132
# =============================================================================
@@ -418,4 +419,4 @@ def main():
418419
print(" [+] Done.")
419420

420421
if __name__ == "__main__":
421-
main()
422+
main()

‎cmd/add.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ var addCmd = &cobra.Command{
1414
Long: `Add one or more URLs to the download queue of a running Surge instance.`,
1515
Run: func(cmd *cobra.Command, args []string) {
1616
// Initialize Global State (needed for config/paths)
17-
initializeGlobalState()
17+
mustInitializeGlobalState()
1818

1919
batchFile, _ := cmd.Flags().GetString("batch")
2020
output, _ := cmd.Flags().GetString("output")

‎cmd/cli_test.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,8 @@ func TestFormatSize_Table(t *testing.T) {
341341
}{
342342
{name: "zero", bytes: 0, want: "0 B"},
343343
{name: "bytes", bytes: 512, want: "512 B"},
344-
{name: "kb", bytes: 1024, want: "1.0 KB"},
345-
{name: "kb-fraction", bytes: 1536, want: "1.5 KB"},
344+
{name: "kb", bytes: 1024, want: "1.0 kB"},
345+
{name: "kb-fraction", bytes: 1536, want: "1.5 kB"},
346346
{name: "mb", bytes: 1024 * 1024, want: "1.0 MB"},
347347
}
348348

@@ -468,7 +468,11 @@ func TestPrintDownloads_FromDatabase_TableAndJSON(t *testing.T) {
468468
if err := json.Unmarshal([]byte(jsonOut), &infos); err != nil {
469469
t.Fatalf("failed to decode json output: %v (out=%q)", err, jsonOut)
470470
}
471-
if len(infos) != 1 || infos[0].ID != entry.ID {
471+
if len(infos) != 1 {
472+
t.Fatalf("expected 1 json entry, got %d: %+v", len(infos), infos)
473+
}
474+
got := infos[0]
475+
if got.ID != entry.ID || got.Filename != entry.Filename || got.Status != entry.Status || got.Downloaded != entry.Downloaded || got.TotalSize != entry.TotalSize || got.Progress != 50 {
472476
t.Fatalf("unexpected JSON payload: %+v", infos)
473477
}
474478
}
@@ -480,8 +484,12 @@ func TestPrintDownloads_JSONEmpty(t *testing.T) {
480484
out := captureStdout(t, func() {
481485
printDownloads(true, "", "", false)
482486
})
483-
if strings.TrimSpace(out) != "[]" {
484-
t.Fatalf("expected empty json array, got %q", strings.TrimSpace(out))
487+
var infos []any
488+
if err := json.Unmarshal([]byte(out), &infos); err != nil {
489+
t.Fatalf("failed to parse json output: %v (out=%q)", err, out)
490+
}
491+
if len(infos) != 0 {
492+
t.Fatalf("expected empty json array, got %d entries: %+v", len(infos), infos)
485493
}
486494
}
487495

@@ -753,21 +761,14 @@ func TestProcessDownloads_RemoteAndLocal(t *testing.T) {
753761

754762
func setupIsolatedCmdState(t *testing.T) {
755763
t.Helper()
756-
tempDir := t.TempDir()
757-
t.Setenv("APPDATA", tempDir)
758-
t.Setenv("USERPROFILE", tempDir)
759-
t.Setenv("XDG_CONFIG_HOME", tempDir)
760-
t.Setenv("HOME", tempDir)
764+
setupXDGEnvIsolation(t)
761765

762766
if err := config.EnsureDirs(); err != nil {
763767
t.Fatalf("EnsureDirs failed: %v", err)
764768
}
765769

766770
state.CloseDB()
767771
state.Configure(filepath.Join(config.GetStateDir(), "surge.db"))
768-
t.Cleanup(func() {
769-
state.CloseDB()
770-
})
771772
}
772773

773774
func captureStdout(t *testing.T, fn func()) string {

0 commit comments

Comments
 (0)