v0.4.0: hardened backends, IsNonCritical, testable CLI#8
Merged
Conversation
Validated on real macOS, Linux GNOME and Windows 11 machines. Library: - Add IsNonCritical(err) + nonCriticalError to distinguish a failed /etc/environment write (no root) from a hard failure on Linux - Linux: write KDE proxy settings via kwriteconfig5 alongside gsettings, so hybrid GNOME/KDE setups work without desktop detection; return the /etc/environment failure as a non-critical error - Windows: clear AutoConfigURL on set/unset so a previous PAC URL no longer persists after switching to manual proxy or unsetting CLI: - Extract dispatch into a testable run(argv, stdout, stderr) int instead of calling os.Exit directly; cmd/sysproxy coverage 12% -> 61% Docs/tests: - README: trim the alternatives table to a single named comparison and document SetPAC vs Get/GetConfig auto-proxy behavior - Add IsNonCritical, CLI run() and example tests - buildinfo: doc comment follows Go convention Total coverage 64.6% -> 72.2%; golangci-lint clean; go test -race green.
Silences the 'Restore cache failed: Dependencies file is not found ... go.sum' warning on every job. The module has no external dependencies, so there is nothing to cache.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR prepares the v0.4.0 release by hardening OS-specific proxy backends, introducing a “non-critical error” classification for best-effort operations, and refactoring the sysproxy CLI to be unit-testable while improving JSON output/docs.
Changes:
- Add
IsNonCritical(err)and use it in Linux global set/unset when/etc/environmentcannot be written/cleared (best-effort behavior). - Improve platform behavior: Windows clears
AutoConfigURLwhen switching to manual proxy; Linux writes GNOME+KDE config without desktop detection; macOS clears stale host/port fields on unset. - Refactor CLI to a
run(argv, stdout, stderr)entrypoint (testable), and adjustget --jsonto emit a per-protocol JSON object.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sysproxy.go | Adds JSON tags to ProxyConfig to support stable --json output keys. |
| sysproxy_windows.go | Clears AutoConfigURL when setting/unsetting manual proxy modes. |
| sysproxy_linux.go | Removes desktop detection; applies GNOME+KDE config when tools exist; wraps /etc/environment failures as non-critical. |
| sysproxy_darwin.go | Clears proxy host/port fields on unset to avoid stale values in System Settings UI. |
| README.md | Documents PAC vs manual reporting behavior and updated Linux/non-critical semantics; adds comparison table. |
| internal/buildinfo/buildinfo.go | Clarifies which variables are injected via -ldflags. |
| example_test.go | Adds additional API usage examples (Get, GetConfig, SetPAC). |
| errors.go | Introduces nonCriticalError and exported IsNonCritical. |
| errors_test.go | Adds unit tests validating non-critical error detection and unwrap behavior. |
| cmd/sysproxy/main.go | Refactors CLI into a testable run function; updates JSON output shape for get. |
| cmd/sysproxy/main_test.go | Updates CLI integration tests for the new JSON output shape. |
| cmd/sysproxy/helpers_test.go | Adds unit tests for CLI helpers and run dispatch/error paths. |
| .gitignore | Ignores local smoke-test artifacts. |
| .github/workflows/test.yml | Disables setup-go caching and removes Codecov token usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+146
to
+160
| if isAvailable("gsettings") { | ||
| if cfg.HTTP != "" { | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy", "mode", "manual") | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy.http", "host", hostFromURL(cfg.HTTP)) | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy.http", "port", portFromURL(cfg.HTTP)) | ||
| } | ||
| if cfg.HTTPS != "" { | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy.https", "host", hostFromURL(cfg.HTTPS)) | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy.https", "port", portFromURL(cfg.HTTPS)) | ||
| } | ||
| if cfg.SOCKS != "" { | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy.socks", "host", hostFromURL(cfg.SOCKS)) | ||
| _ = runGsettings(ctx, "set", "org.gnome.system.proxy.socks", "port", portFromURL(cfg.SOCKS)) | ||
| } | ||
| } |
Comment on lines
137
to
+141
| if err := sysproxy.SetContext(ctx, proxyURL, scope); err != nil { | ||
| dieJSON(jsonOut, "set failed: "+err.Error()) | ||
| return dieJSON(jsonOut, "set failed: "+err.Error(), stdout, stderr) | ||
| } | ||
| printOK(jsonOut, map[string]any{"proxy": proxyURL, "scope": scope.String()}) | ||
| printOK(jsonOut, map[string]any{"proxy": proxyURL, "scope": scope.String()}, stdout) | ||
| return 0 |
Comment on lines
171
to
+175
| if err := sysproxy.UnsetContext(ctx, scope); err != nil { | ||
| dieJSON(jsonOut, "unset failed: "+err.Error()) | ||
| return dieJSON(jsonOut, "unset failed: "+err.Error(), stdout, stderr) | ||
| } | ||
| printOK(jsonOut, map[string]any{"scope": scope.String()}) | ||
| printOK(jsonOut, map[string]any{"scope": scope.String()}, stdout) | ||
| return 0 |
Comment on lines
+122
to
127
| if _, hasErr := m["error"]; hasErr { | ||
| return | ||
| } | ||
| if _, hasHTTP := m["http"]; !hasHTTP { | ||
| t.Errorf("JSON output missing 'http' or 'error' key: %v", m) | ||
| } |
Comment on lines
+14
to
+33
| func captureStdout(t *testing.T, fn func()) string { | ||
| t.Helper() | ||
| orig := os.Stdout | ||
| r, w, err := os.Pipe() | ||
| if err != nil { | ||
| t.Fatalf("pipe: %v", err) | ||
| } | ||
| os.Stdout = w | ||
| t.Cleanup(func() { | ||
| os.Stdout = orig | ||
| }) | ||
|
|
||
| fn() | ||
| _ = w.Close() | ||
|
|
||
| var buf bytes.Buffer | ||
| _, _ = io.Copy(&buf, r) | ||
| _ = r.Close() | ||
| return buf.String() | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Validated on macOS, Linux GNOME and Windows 11. Windows AutoConfigURL fix, KDE write, IsNonCritical, CLI refactor (coverage 64.6%→72.2%), README/GoDoc cleanup, CI cache fix.