Fix #615 - Test cases and cleanup

code/sonalyze/cmd/parse/parse.go

@@ -226,9 +226,6 @@ type SFS = SimpleFormatSpec
 type AFS = SimpleFormatSpecWithAttr
 type ZFA = SynthesizedFormatSpecWithAttr
 
-// TODO: IMPROVEME: The defaulted fields here follow the Rust code.  Now that it's trivial to do so,
-// we should consider adding more.
-//
 // TODO: IMPROVEME: The use of utc for "localtime" is a bug that comes from the Rust code.
 
 var parseFormatters = DefineTableFromMap(
@@ -242,25 +239,25 @@ var parseFormatters = DefineTableFromMap(
 		"MemtotalKB": SFS{"Installed main memory", ""},
 		"memtotal":   ZFA{"Installed main memory (GB)", "MemtotalKB", FmtDivideBy1M},
 		"User":       SFS{"Username of process owner", "user"},
-		"Pid":        AFS{"Process ID", "pid", FmtDefaultable},
-		"Ppid":       AFS{"Process parent ID", "ppid", FmtDefaultable},
+		"Pid":        SFS{"Process ID", "pid"},
+		"Ppid":       SFS{"Process parent ID", "ppid"},
 		"Job":        SFS{"Job ID", "job"},
 		"Cmd":        SFS{"Command name", "cmd"},
 		"CpuPct":     SFS{"cpu% reading (CONSULT DOCUMENTATION)", "cpu_pct"},
 		"CpuKB":      SFS{"Virtual memory reading", "cpukib"},
 		"mem_gb":     ZFA{"Virtual memory reading", "CpuKB", FmtDivideBy1M},
 		"RssAnonKB":  SFS{"RssAnon reading", ""},
 		"res_gb":     ZFA{"RssAnon reading", "RssAnonKB", FmtDivideBy1M},
-		"Gpus":       AFS{"GPU set (`none`,`unknown`,list)", "gpus", FmtDefaultable},
-		"GpuPct":     AFS{"GPU utilization reading", "gpu_pct", FmtDefaultable},
-		"GpuMemPct":  AFS{"GPU memory percentage reading", "gpumem_pct", FmtDefaultable},
-		"GpuKB":      AFS{"GPU memory utilization reading", "gpukib", FmtDefaultable},
-		"gpumem_gb":  ZFA{"GPU memory utilization reading", "GpuKB", FmtDivideBy1M | FmtDefaultable},
-		"GpuFail":    AFS{"GPU status flag (0=ok, 1=error state)", "gpu_status", FmtDefaultable},
-		"CpuTimeSec": AFS{"CPU time since last reading (seconds, CONSULT DOCUMENTATION)", "cputime_sec", FmtDefaultable},
-		"Rolledup":   AFS{"Number of rolled-up processes, minus 1", "rolledup", FmtDefaultable},
+		"Gpus":       SFS{"GPU set (`none`,`unknown`,list)", "gpus"},
+		"GpuPct":     SFS{"GPU utilization reading", "gpu_pct"},
+		"GpuMemPct":  SFS{"GPU memory percentage reading", "gpumem_pct"},
+		"GpuKB":      SFS{"GPU memory utilization reading", "gpukib"},
+		"gpumem_gb":  ZFA{"GPU memory utilization reading", "GpuKB", FmtDivideBy1M},
+		"GpuFail":    SFS{"GPU status flag (0=ok, 1=error state)", "gpu_status"},
+		"CpuTimeSec": SFS{"CPU time since last reading (seconds, CONSULT DOCUMENTATION)", "cputime_sec"},
+		"Rolledup":   SFS{"Number of rolled-up processes, minus 1", "rolledup"},
 		"Flags":      SFS{"Bit vector of flags, UTSL", ""},
-		"CpuUtilPct": AFS{"CPU utilization since last reading (percent, CONSULT DOCUMENTATION)", "cpu_util_pct", FmtDefaultable},
+		"CpuUtilPct": SFS{"CPU utilization since last reading (percent, CONSULT DOCUMENTATION)", "cpu_util_pct"},
 	},
 )
 

code/sonalyze/cmd/profile/print.go

@@ -71,29 +71,22 @@ func (pc *ProfileCommand) printProfile(
 	m *profData,
 	processes sonarlog.SampleStreams,
 ) error {
-	if hasRolledup {
-		// Add the "nproc" / "NumProcs" field if it is required and the fields are still the
-		// defaults.  This is pretty dumb (but compatible with the Rust code); it gets even dumber
-		// with two versions of the default fields string.
-		currFields := strings.Join(
-			uslices.Map(
-				pc.PrintFields,
-				func(fs FieldSpec) string { return fs.Name },
-			),
-			",",
+	// Add the "nproc" / "NumProcs" field if it is required (we can't do it until rollup has
+	// happened) and the fields are still the defaults.  This is not quite compatible with older
+	// sonalyze: here we have default fields only if no fields were specified (defaults were
+	// applied), while in older code a field list identical to the default would be taken as having
+	// default fields too.  The new logic is better.
+	//
+	// Anyway, this hack depends on nothing interesting having happened to pc.Fmt or pc.PrintFields
+	// after the initial parsing.
+	hasDefaultFields := pc.Fmt == ""
+	if hasRolledup && hasDefaultFields {
+		pc.PrintFields, _, _ = ParseFormatSpec(
+			profileDefaultFieldsWithNproc,
+			"",
+			profileFormatters,
+			profileAliases,
 		)
-		var newFields string
-		if currFields == v0ProfileDefaultFields {
-			newFields = v0ProfileDefaultFieldsWithNproc
-		} else if currFields == v1ProfileDefaultFields {
-			newFields = v1ProfileDefaultFieldsWithNproc
-		}
-		if newFields != "" {
-			pc.PrintFields = uslices.Map(
-				strings.Split(newFields, ","),
-				func(name string) FieldSpec { return FieldSpec{Name: name} },
-			)
-		}
 	}
 
 	if pc.PrintOpts.Csv || pc.PrintOpts.Awk {

code/sonalyze/table/data.go

@@ -43,27 +43,13 @@ func (val IntOrEmpty) String() string {
 	return strconv.FormatInt(int64(val), 10)
 }
 
-// Formatters that take a context value
-
 func FormatDurationValue(secs int64, ctx PrintMods) string {
 	if (ctx & PrintModSec) != 0 {
 		return fmt.Sprint(secs)
 	}
 	return FormatDurationDHM(secs, ctx)
 }
 
-func FormatDateTimeValue(timestamp int64, ctx PrintMods) string {
-	// Note, it is part of the API that PrintModSec takes precedence over PrintModIso (this
-	// simplifies various other paths).
-	if (ctx & PrintModSec) != 0 {
-		return fmt.Sprint(timestamp)
-	}
-	if (ctx & PrintModIso) != 0 {
-		return FormatIsoUtc(timestamp)
-	}
-	return FormatYyyyMmDdHhMmUtc(timestamp)
-}
-
 // The DurationValue had two different formats in older code: %2dd%2dh%2dm and %dd%2dh%2dm.  The
 // embedded spaces made things line up properly in fixed-format outputs of jobs, and most scripts
 // would likely use "duration/sec" etc instead, but the variability in the leading space is weird
@@ -91,6 +77,18 @@ func FormatDurationDHM(durationSec int64, ctx PrintMods) string {
 	return fmt.Sprintf("%dd%dh%dm", days, hours, minutes)
 }
 
+func FormatDateTimeValue(timestamp int64, ctx PrintMods) string {
+	// Note, it is part of the API that PrintModSec takes precedence over PrintModIso (this
+	// simplifies various other paths).
+	if (ctx & PrintModSec) != 0 {
+		return fmt.Sprint(timestamp)
+	}
+	if (ctx & PrintModIso) != 0 {
+		return FormatIsoUtc(timestamp)
+	}
+	return FormatYyyyMmDdHhMmUtc(timestamp)
+}
+
 func FormatYyyyMmDdHhMmUtc(t int64) string {
 	return time.Unix(t, 0).UTC().Format("2006-01-02 15:04")
 }

code/sonalyze/table/data_test.go

@@ -0,0 +1,49 @@
+package table
+
+import (
+	"fmt"
+	"testing"
+)
+
+const (
+	now = 1732518173          // 2024-11-25T07:02:53Z
+	dur = 3600*33 + 60*6 + 38 // 1d 9h 7m, rounded up
+)
+
+func TestDataFormatting(t *testing.T) {
+	if s := DateValue(now).String(); s != "2024-11-25" {
+		t.Fatalf("DateValue %s", s)
+	}
+	if s := TimeValue(now).String(); s != "07:02" {
+		t.Fatalf("TimeValue %s", s)
+	}
+
+	if s := IntOrEmpty(7).String(); s != "7" {
+		t.Fatalf("IntOrEmpty %s", s)
+	}
+	if s := IntOrEmpty(0).String(); s != "" {
+		t.Fatalf("IntOrEmpty %s", s)
+	}
+
+	if s := FormatDurationValue(dur, 0); s != "1d9h7m" {
+		t.Fatalf("Duration %s", s)
+	}
+	if s := FormatDurationValue(dur, PrintModFixed); s != " 1d 9h 7m" {
+		t.Fatalf("Duration %s", s)
+	}
+	if s := FormatDurationValue(dur, PrintModSec); s != fmt.Sprint(dur) {
+		t.Fatalf("Duration %s", s)
+	}
+
+	if s := FormatDateTimeValue(now, 0); s != "2024-11-25 07:02" {
+		t.Fatalf("DateTimeValue %s", s)
+	}
+	if s := FormatDateTimeValue(now, PrintModSec|PrintModIso); s != fmt.Sprint(now) {
+		t.Fatalf("DateTimeValue %s", s)
+	}
+	if s := FormatDateTimeValue(now, PrintModIso); s != "2024-11-25T07:02:53Z" {
+		t.Fatalf("DateTimeValue %s", s)
+	}
+
+	// For the other types, the formatters are all embedded in the reflection code.
+}

code/sonalyze/table/format.go

@@ -492,11 +492,9 @@ func formatAwk(unbufOut io.Writer, fields []FieldSpec, opts *FormatOptions, cols
 		sep := ""
 		for col := range fields {
 			val := cols[col][row]
-			if !(opts.NoDefaults && val == "*skip*") {
-				line.WriteString(sep)
-				line.WriteString(strings.ReplaceAll(val, " ", "_"))
-				sep = " "
-			}
+			line.WriteString(sep)
+			line.WriteString(strings.ReplaceAll(val, " ", "_"))
+			sep = " "
 		}
 		if opts.Tag != "" {
 			line.WriteString(sep)