Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion daemon/handlers.track_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package daemon

import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
Expand All @@ -23,6 +24,8 @@ type fakeCommandStore struct {

cursorSetCalls int
pruneCalls int

engine string
}

func (f *fakeCommandStore) SavePre(ctx context.Context, cmd model.Command, rt time.Time) error {
Expand Down Expand Up @@ -72,6 +75,13 @@ func (f *fakeCommandStore) Prune(ctx context.Context, cursor time.Time) error {
return nil
}

func (f *fakeCommandStore) Engine() string {
if f.engine == "" {
return model.StorageEngineFile
}
return f.engine
}

func (f *fakeCommandStore) Close() error { return nil }

// fakeConfigService implements model.ConfigService without mockery.
Expand Down Expand Up @@ -184,12 +194,14 @@ func (s *TrackHandlerTestSuite) TestTrackPostNotEnoughToFlush() {
}

func (s *TrackHandlerTestSuite) TestTrackPostFlushSyncsAndPrunes() {
var sentPayload model.PostTrackArgs
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_ = json.NewDecoder(r.Body).Decode(&sentPayload)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Ignoring the error from 'json.NewDecoder().Decode()' can lead to silent test failures or confusing assertion errors if the request body is malformed or empty. It is better to explicitly assert that decoding succeeded.

Suggested change
_ = json.NewDecoder(r.Body).Decode(&sentPayload)
err := json.NewDecoder(r.Body).Decode(&sentPayload)
assert.NoError(s.T(), err)

w.WriteHeader(http.StatusNoContent)
}))
defer server.Close()

store := &fakeCommandStore{noCursorExist: true}
store := &fakeCommandStore{noCursorExist: true, engine: model.StorageEngineBolt}
commandStore = store
stConfig = fakeConfigService{cfg: model.ShellTimeConfig{Token: "t", APIEndpoint: server.URL, FlushCount: 1}}

Expand All @@ -204,6 +216,9 @@ func (s *TrackHandlerTestSuite) TestTrackPostFlushSyncsAndPrunes() {
assert.Len(s.T(), store.post, 1)
assert.Equal(s.T(), 1, store.cursorSetCalls)
assert.Equal(s.T(), 1, store.pruneCalls)
// the payload must report the engine that buffered the commands
assert.Equal(s.T(), model.StorageEngineBolt, sentPayload.Meta.CliEngine)
assert.Equal(s.T(), 1, sentPayload.Meta.Source, "daemon path must mark source as daemon")
}

func (s *TrackHandlerTestSuite) TestTrackPostFallsBackToFileStore() {
Expand Down
4 changes: 4 additions & 0 deletions model/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ type TrackingMetaData struct {
Terminal string `json:"terminal,omitempty"`
Multiplexer string `json:"multiplexer,omitempty"`

// CliEngine is the storage engine that buffered these commands:
// StorageEngineFile or StorageEngineBolt.
CliEngine string `json:"cliEngine,omitempty"`

// 0: cli, 1: daemon
Source int `json:"source"`
}
Expand Down
5 changes: 5 additions & 0 deletions model/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ type CommandStore interface {
// before the cursor), keeping unfinished pre commands.
Prune(ctx context.Context, cursor time.Time) error

// Engine reports which storage engine backs this store (StorageEngineFile
// or StorageEngineBolt). It is attached to sync metadata so the server
// knows how the commands were buffered.
Engine() string

// Close releases any resources held by the store (no-op for fileStore).
Close() error
}
Expand Down
2 changes: 2 additions & 0 deletions model/store_bolt.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ func (s *boltStore) Prune(ctx context.Context, cursor time.Time) error {
})
}

func (s *boltStore) Engine() string { return StorageEngineBolt }

func (s *boltStore) Close() error {
if s.db == nil {
return nil
Expand Down
2 changes: 2 additions & 0 deletions model/store_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,6 @@ func (s *fileStore) Prune(ctx context.Context, cursor time.Time) error {
return os.WriteFile(GetCursorFilePath(), []byte(fmt.Sprintf("%d", cursor.UnixNano())), 0644)
}

func (s *fileStore) Engine() string { return StorageEngineFile }

func (s *fileStore) Close() error { return nil }
1 change: 1 addition & 0 deletions model/tracking_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func BuildTrackingData(ctx context.Context, store CommandStore, config ShellTime
meta := TrackingMetaData{
OS: sysInfo.Os,
OSVersion: sysInfo.Version,
CliEngine: store.Engine(),
}

trackingData := make([]TrackingData, 0)
Expand Down
22 changes: 22 additions & 0 deletions model/tracking_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ func TestBuildTrackingData(t *testing.T) {
require.Equal(t, 42, res.Data[0].PPID)
require.Equal(t, "h", res.Meta.Hostname)
require.Equal(t, "bash", res.Meta.Shell)
require.Equal(t, StorageEngineBolt, res.Meta.CliEngine)
}

func TestBuildTrackingDataFileEngine(t *testing.T) {
t.Setenv("HOME", t.TempDir())
InitFolder("") // reset globals to the default .shelltime under the temp HOME
Comment on lines +39 to +40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the 'HOME' environment variable is platform-dependent and violates the general rule of avoiding environment variables like '$HOME' for path resolution. It will fail on Windows where 'os.UserHomeDir()' looks up 'USERPROFILE'. Since 'InitFolder' accepts a custom path, pass 't.TempDir()' directly to it to ensure platform independence.

Suggested change
t.Setenv("HOME", t.TempDir())
InitFolder("") // reset globals to the default .shelltime under the temp HOME
InitFolder(t.TempDir())
References
  1. For platform-independent paths, use filepath.Join to combine segments and os.UserHomeDir() to get the home directory, rather than hardcoding path separators or environment variables like $HOME.


store := NewFileStore()
defer store.Close()

ctx := context.Background()
start := time.Now()
cmd := Command{Shell: "zsh", SessionID: 3, Command: "ls -la", Username: "u", Hostname: "h", Time: start}
require.NoError(t, store.SavePre(ctx, cmd, start))
post := cmd
post.Time = start.Add(time.Second)
require.NoError(t, store.SavePost(ctx, post, 0, post.Time))

res, err := BuildTrackingData(ctx, store, ShellTimeConfig{})
require.NoError(t, err)
require.Len(t, res.Data, 1)
require.Equal(t, StorageEngineFile, res.Meta.CliEngine)
}

func TestBuildTrackingDataExcludes(t *testing.T) {
Expand Down
Loading