Skip to content

Commit e30b750

Browse files
cocosipcocosip
committed
♻️ refactor(app): extract magic strings into named constants
- Add doctorJSONFlag and stringNone constants in doctor.go - Refactor fallbackString to use shared stringNone constant - Extract test constants in import_follow_test.go and import_ingest_test.go - Create helper assertion functions to reduce test duplication - Add revive:disable:var-naming comments for packages with naming conventions
1 parent caa09da commit e30b750

8 files changed

Lines changed: 183 additions & 140 deletions

File tree

‎internal/app/doctor.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,18 @@ type doctorMCPReport struct {
9292
ToolCount int `json:"tool_count"`
9393
}
9494

95+
const (
96+
doctorJSONFlag = "--json"
97+
stringNone = "none"
98+
)
99+
95100
func parseDoctorOptions(args []string) (doctorOptions, error) {
96101
var options doctorOptions
97102
for _, arg := range args {
98103
switch strings.TrimSpace(arg) {
99104
case "":
100105
continue
101-
case "--json":
106+
case doctorJSONFlag:
102107
options.JSON = true
103108
default:
104109
return doctorOptions{}, fmt.Errorf("unknown doctor flag %q", arg)
@@ -251,7 +256,7 @@ func stringPointerOrNil(value string) *string {
251256

252257
func pointerStringOrNone(value *string) string {
253258
if value == nil {
254-
return "none"
259+
return stringNone
255260
}
256261
return *value
257262
}

‎internal/app/import_follow.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -776,16 +776,16 @@ func formatFollowImportsReport(report followImportsReport) string {
776776
fmt.Sprintf("source=%s", report.Source),
777777
fmt.Sprintf("input=%s", report.Input),
778778
fmt.Sprintf("state_file=%s", report.StateFile),
779-
fmt.Sprintf("requested_watch_mode=%s", fallbackString(report.RequestedWatchMode, "none")),
780-
fmt.Sprintf("active_watch_mode=%s", fallbackString(report.ActiveWatchMode, "none")),
779+
fmt.Sprintf("requested_watch_mode=%s", fallbackString(report.RequestedWatchMode)),
780+
fmt.Sprintf("active_watch_mode=%s", fallbackString(report.ActiveWatchMode)),
781781
fmt.Sprintf("watch_fallbacks=%d", report.WatchFallbacks),
782-
fmt.Sprintf("last_fallback_reason=%s", fallbackString(report.LastFallbackReason, "none")),
782+
fmt.Sprintf("last_fallback_reason=%s", fallbackString(report.LastFallbackReason)),
783783
fmt.Sprintf("offset=%d", report.Offset),
784784
fmt.Sprintf("consumed_bytes=%d", report.ConsumedBytes),
785785
fmt.Sprintf("pending_bytes=%d", report.PendingBytes),
786786
fmt.Sprintf("truncated=%t", report.Truncated),
787787
fmt.Sprintf("checkpoint_reset=%t", report.CheckpointReset),
788-
fmt.Sprintf("reset_reason=%s", fallbackString(report.ResetReason, "none")),
788+
fmt.Sprintf("reset_reason=%s", fallbackString(report.ResetReason)),
789789
}
790790
if report.Batch != nil {
791791
lines = append(lines,
@@ -796,8 +796,8 @@ func formatFollowImportsReport(report followImportsReport) string {
796796
fmt.Sprintf("failed=%d", report.Batch.Failed),
797797
fmt.Sprintf("materialized=%d", report.Batch.Materialized),
798798
fmt.Sprintf("suppressed=%d", report.Batch.Suppressed),
799-
fmt.Sprintf("failed_output=%s", fallbackString(report.Batch.FailedOutput, "none")),
800-
fmt.Sprintf("failed_manifest=%s", fallbackString(report.Batch.FailedManifest, "none")),
799+
fmt.Sprintf("failed_output=%s", fallbackString(report.Batch.FailedOutput)),
800+
fmt.Sprintf("failed_manifest=%s", fallbackString(report.Batch.FailedManifest)),
801801
)
802802
}
803803
if report.BatchError != nil {

‎internal/app/import_follow_test.go‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ import (
1212
"github.com/fsnotify/fsnotify"
1313
)
1414

15+
const (
16+
followImportsWatcherSource = "watcher_import"
17+
followImportsFirstEvent = `{"external_id":"watcher:1","type":"discovery","title":"First","content":"First event.","importance":4}`
18+
)
19+
1520
func TestParseFollowImportsOptions(t *testing.T) {
1621
options, err := parseFollowImportsOptions([]string{
17-
"--source", "watcher_import",
22+
"--source", followImportsWatcherSource,
1823
"--input", "events.jsonl",
1924
"--state-file", "events.offset.json",
2025
"--failed-output", "failed.jsonl",
@@ -31,7 +36,7 @@ func TestParseFollowImportsOptions(t *testing.T) {
3136
if err != nil {
3237
t.Fatalf("parseFollowImportsOptions: %v", err)
3338
}
34-
if got, want := string(options.Source), "watcher_import"; got != want {
39+
if got, want := string(options.Source), followImportsWatcherSource; got != want {
3540
t.Fatalf("source mismatch: got %q want %q", got, want)
3641
}
3742
if got, want := options.InputPath, "events.jsonl"; got != want {
@@ -55,7 +60,7 @@ func TestParseFollowImportsOptions(t *testing.T) {
5560
}
5661

5762
func TestParseFollowImportsOptionsRejectsMissingInput(t *testing.T) {
58-
_, err := parseFollowImportsOptions([]string{"--source", "watcher_import"})
63+
_, err := parseFollowImportsOptions([]string{"--source", followImportsWatcherSource})
5964
if err == nil {
6065
t.Fatal("expected missing input error")
6166
}
@@ -66,7 +71,7 @@ func TestParseFollowImportsOptionsRejectsMissingInput(t *testing.T) {
6671

6772
func TestParseFollowImportsOptionsRejectsInvalidWatchMode(t *testing.T) {
6873
_, err := parseFollowImportsOptions([]string{
69-
"--source", "watcher_import",
74+
"--source", followImportsWatcherSource,
7075
"--input", "events.jsonl",
7176
"--watch-mode", "interrupts",
7277
})
@@ -182,14 +187,14 @@ func TestAppFollowImportsOnceConsumesOnlyCompleteLinesAndPersistsCheckpoint(t *t
182187

183188
eventsPath := filepath.Join(root, "events.jsonl")
184189
statePath := filepath.Join(root, "events.offset.json")
185-
first := `{"external_id":"watcher:1","type":"discovery","title":"First","content":"First event.","importance":4}`
190+
first := followImportsFirstEvent
186191
secondPrefix := `{"external_id":"watcher:2","type":"todo","title":"Second"`
187192
if err := os.WriteFile(eventsPath, []byte(first+"\n"+secondPrefix), 0o644); err != nil {
188193
t.Fatalf("WriteFile events: %v", err)
189194
}
190195

191196
report, err := instance.FollowImportsOnce(context.Background(), FollowImportsInput{
192-
Source: "watcher_import",
197+
Source: followImportsWatcherSource,
193198
InputPath: eventsPath,
194199
StatePath: statePath,
195200
CWD: root,
@@ -234,7 +239,7 @@ func TestAppFollowImportsOnceConsumesOnlyCompleteLinesAndPersistsCheckpoint(t *t
234239
}
235240

236241
report, err = instance.FollowImportsOnce(context.Background(), FollowImportsInput{
237-
Source: "watcher_import",
242+
Source: followImportsWatcherSource,
238243
InputPath: eventsPath,
239244
StatePath: statePath,
240245
CWD: root,
@@ -276,7 +281,7 @@ func TestAppFollowImportsOnceUsesCheckpointRecoveryWhenNoNewLinesExist(t *testin
276281
cfg := ingestTestConfig(root)
277282
eventsPath := filepath.Join(root, "events.jsonl")
278283
statePath := filepath.Join(root, "events.offset.json")
279-
event := `{"external_id":"watcher:1","type":"discovery","title":"First","content":"First event.","importance":4}`
284+
event := followImportsFirstEvent
280285
if err := os.WriteFile(eventsPath, []byte(event+"\n"), 0o644); err != nil {
281286
t.Fatalf("WriteFile events: %v", err)
282287
}
@@ -286,7 +291,7 @@ func TestAppFollowImportsOnceUsesCheckpointRecoveryWhenNoNewLinesExist(t *testin
286291
t.Fatalf("New first instance: %v", err)
287292
}
288293
report, err := instance.FollowImportsOnce(context.Background(), FollowImportsInput{
289-
Source: "watcher_import",
294+
Source: followImportsWatcherSource,
290295
InputPath: eventsPath,
291296
StatePath: statePath,
292297
CWD: root,
@@ -311,7 +316,7 @@ func TestAppFollowImportsOnceUsesCheckpointRecoveryWhenNoNewLinesExist(t *testin
311316
defer func() { _ = instance.Close() }()
312317

313318
report, err = instance.FollowImportsOnce(context.Background(), FollowImportsInput{
314-
Source: "watcher_import",
319+
Source: followImportsWatcherSource,
315320
InputPath: eventsPath,
316321
StatePath: statePath,
317322
CWD: root,
@@ -353,12 +358,12 @@ func TestAppFollowImportsOnceResetsOffsetAfterTruncation(t *testing.T) {
353358

354359
eventsPath := filepath.Join(root, "events.jsonl")
355360
statePath := filepath.Join(root, "events.offset.json")
356-
first := `{"external_id":"watcher:1","type":"discovery","title":"First","content":"First event.","importance":4}`
361+
first := followImportsFirstEvent
357362
if err := os.WriteFile(eventsPath, []byte(first+"\n"), 0o644); err != nil {
358363
t.Fatalf("WriteFile first events: %v", err)
359364
}
360365
if _, err := instance.FollowImportsOnce(context.Background(), FollowImportsInput{
361-
Source: "watcher_import",
366+
Source: followImportsWatcherSource,
362367
InputPath: eventsPath,
363368
StatePath: statePath,
364369
CWD: root,
@@ -373,7 +378,7 @@ func TestAppFollowImportsOnceResetsOffsetAfterTruncation(t *testing.T) {
373378
}
374379

375380
report, err := instance.FollowImportsOnce(context.Background(), FollowImportsInput{
376-
Source: "watcher_import",
381+
Source: followImportsWatcherSource,
377382
InputPath: eventsPath,
378383
StatePath: statePath,
379384
CWD: root,
@@ -423,7 +428,7 @@ func TestAppFollowImportsOnceWritesBatchScopedFailureExports(t *testing.T) {
423428
}
424429

425430
report, err := instance.FollowImportsOnce(context.Background(), FollowImportsInput{
426-
Source: "watcher_import",
431+
Source: followImportsWatcherSource,
427432
InputPath: eventsPath,
428433
StatePath: statePath,
429434
CWD: root,
@@ -506,7 +511,7 @@ func TestAppFollowImportsOnceResetsOffsetWhenFileIsReplacedWithoutShrinking(t *t
506511
}
507512

508513
report, err := instance.FollowImportsOnce(context.Background(), FollowImportsInput{
509-
Source: "watcher_import",
514+
Source: followImportsWatcherSource,
510515
InputPath: eventsPath,
511516
StatePath: statePath,
512517
CWD: root,
@@ -524,7 +529,7 @@ func TestAppFollowImportsOnceResetsOffsetWhenFileIsReplacedWithoutShrinking(t *t
524529
}
525530

526531
report, err = instance.FollowImportsOnce(context.Background(), FollowImportsInput{
527-
Source: "watcher_import",
532+
Source: followImportsWatcherSource,
528533
InputPath: eventsPath,
529534
StatePath: statePath,
530535
CWD: root,

‎internal/app/import_ingest.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,9 +580,9 @@ func formatIngestImportsReport(report ingestImportsReport) string {
580580
fmt.Sprintf("status=%s", report.Status),
581581
fmt.Sprintf("source=%s", report.Source),
582582
fmt.Sprintf("input=%s", report.Input),
583-
fmt.Sprintf("failed_output=%s", fallbackString(report.FailedOutput, "none")),
583+
fmt.Sprintf("failed_output=%s", fallbackString(report.FailedOutput)),
584584
fmt.Sprintf("failed_output_written=%d", report.FailedOutputWritten),
585-
fmt.Sprintf("failed_manifest=%s", fallbackString(report.FailedManifest, "none")),
585+
fmt.Sprintf("failed_manifest=%s", fallbackString(report.FailedManifest)),
586586
fmt.Sprintf("failed_manifest_count=%d", report.FailedManifestCount),
587587
fmt.Sprintf("session_id=%s", report.Session.ID),
588588
fmt.Sprintf("resolved_by=%s", report.Scope.ResolvedBy),
@@ -636,10 +636,10 @@ func writeIngestFailureManifest(path string, report ingestImportsReport, failure
636636
return nil
637637
}
638638

639-
func fallbackString(value string, fallback string) string {
639+
func fallbackString(value string) string {
640640
value = strings.TrimSpace(value)
641641
if value == "" {
642-
return fallback
642+
return stringNone
643643
}
644644
return value
645645
}

0 commit comments

Comments
 (0)