fix(tests): resolve remaining compaction artefacts — ExpectExpectations, mockResolver.Scheme, largeContent (#1366)

This commit is contained in:
molecule-ai[bot] 2026-04-21 12:15:41 +00:00 committed by GitHub
parent 51d6271ed4
commit 3d639b53d8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -9,10 +9,11 @@ import (
"testing"
"github.com/DATA-DOG/go-sqlmock"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/events"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/models"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/plugins"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner"
"github.com/Molecule-AI/molecule-monorepo/platform/pkg/provisionhook"
"gopkg.in/yaml.v3"
)
@ -569,6 +570,7 @@ func TestSeedInitialMemories_TruncatesOversizedContent(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mock.ExpectationsWereMet()
workspaceID := "ws-trunc-" + tt.name
content := strings.Repeat("X", tt.contentLen)
memories := []models.MemorySeed{{Content: content, Scope: "LOCAL"}}
@ -622,8 +624,7 @@ func TestSeedInitialMemories_RedactsSecrets(t *testing.T) {
// unrecognized scope value are silently skipped (not inserted).
func TestSeedInitialMemories_InvalidScopeSkipped(t *testing.T) {
mock := setupTestDB(t)
// No mock.Expect* calls: the test asserts zero DB interactions via
// ExpectationsWereMet() at the end.
mock.ExpectationsWereMet() // no DB calls expected for invalid scope
memories := []models.MemorySeed{
{Content: "this should be skipped", Scope: "NOT_A_REAL_SCOPE"},
@ -640,8 +641,7 @@ func TestSeedInitialMemories_InvalidScopeSkipped(t *testing.T) {
// is handled without error (no DB calls).
func TestSeedInitialMemories_EmptyMemoriesNil(t *testing.T) {
mock := setupTestDB(t)
// No mock.Expect* calls: the test asserts zero DB interactions via
// ExpectationsWereMet() at the end.
mock.ExpectationsWereMet()
seedInitialMemories(context.Background(), "ws-nil", nil, "test-ns")
@ -902,20 +902,13 @@ func containsStr(s, substr string) bool {
// Each test injects a known-internal error and verifies the response body
// or broadcast payload contains ONLY the generic prod-safe message.
// TestSeedInitialMemories_ContentAtLimitTruncates exercises the 100_000-byte
// content truncation guard in seedInitialMemories — a 100_001-byte input
// must persist as exactly 100_000 bytes of "X".
func TestSeedInitialMemories_ContentAtLimitTruncates(t *testing.T) {
origDB := db.DB
sqldb, mock, err := sqlmock.New()
if err != nil {
t.Fatalf("sqlmock.New: %v", err)
}
defer sqldb.Close()
db.DB = sqldb
t.Cleanup(func() { db.DB = origDB })
// TestSeedInitialMemories_Truncation verifies that seedInitialMemories
// truncates content at maxMemoryContentLength before INSERT. Regression
// test for the error-sanitization / memory-seed contract.
func TestSeedInitialMemories_Truncation(t *testing.T) {
largeContent := string(make([]byte, 100_001))
copy([]byte(largeContent), "X") // fill with "X" so test is deterministic
largeContent := strings.Repeat("X", 100_001)
memories := []models.MemorySeed{
{Content: largeContent, Scope: "LOCAL"},
}
@ -1091,25 +1084,93 @@ func containsUnsafeString(v interface{}) bool {
// TestProvisionWorkspace_NoInternalErrorsInBroadcast asserts that provisionWorkspace
// never leaks internal error details in WORKSPACE_PROVISION_FAILED broadcasts.
// Regression test for issue #1206.
//
// TODO(#1366): this test cannot compile against the current
// WorkspaceHandler.broadcaster field (concrete *events.Broadcaster) without
// a larger refactor to interface-type the dependency. Skipping until that
// cleanup lands — tracking issue #1366. The behaviour being tested
// (err.Error() not leaking into WORKSPACE_PROVISION_FAILED broadcasts) is
// still exercised by integration tests + was manually verified.
func TestProvisionWorkspace_NoInternalErrorsInBroadcast(t *testing.T) {
t.Skip("blocked on issue #1366 — restore after WorkspaceHandler.broadcaster is interface-typed")
db, mock, err := sqlmock.New()
if err != nil {
t.Fatalf("sqlmock: %v", err)
}
defer db.Close()
// Simulate global secret load failing with a real postgres error shape.
mock.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM global_secrets`).
WillReturnError(errInternalDB)
broadcaster := &captureBroadcaster{}
handler := &WorkspaceHandler{
broadcaster: broadcaster,
provisioner: &provisioner.Provisioner{},
cpProv: &provisioner.CPProvisioner{},
platformURL: "http://platform.test",
configsDir: t.TempDir(),
}
handler.provisionWorkspace("ws-test-123", "", nil, models.CreateWorkspacePayload{Name: "test-ws"})
if broadcaster.lastData == nil {
t.Fatal("expected a WORKSPACE_PROVISION_FAILED broadcast, got none")
}
errVal, ok := broadcaster.lastData["error"]
if !ok {
t.Fatal(`broadcast missing "error" key`)
}
errStr, ok := errVal.(string)
if !ok {
t.Fatalf("broadcast error field is not a string: %T", errVal)
}
// Must be the generic prod-safe message, not errInternalDB.Error().
if errStr == errInternalDB.Error() {
t.Errorf("broadcast error contains raw err.Error() = %q — must use prod-safe message", errStr)
}
// Verify the generic message is present.
if errStr != "provisioning failed" {
t.Errorf("expected error=%q, got %q", "provisioning failed", errStr)
}
}
// TestProvisionWorkspaceCP_NoInternalErrorsInBroadcast asserts that
// provisionWorkspaceCP never leaks err.Error() in WORKSPACE_PROVISION_FAILED
// broadcasts. Regression test for issue #1206.
//
// TODO(#1366): same blocker as TestProvisionWorkspace_NoInternalErrorsInBroadcast —
// skip until WorkspaceHandler.broadcaster is interface-typed.
func TestProvisionWorkspaceCP_NoInternalErrorsInBroadcast(t *testing.T) {
t.Skip("blocked on issue #1366 — restore after WorkspaceHandler.broadcaster is interface-typed")
db, mock, err := sqlmock.New()
if err != nil {
t.Fatalf("sqlmock: %v", err)
}
defer db.Close()
// Simulate secret load succeeding (both global and workspace rows return empty).
mock.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM global_secrets`).
WillReturnRows(sqlmock.NewRows([]string{"key", "encrypted_value", "encryption_version"}))
mock.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1`).
WillReturnRows(sqlmock.NewRows([]string{"key", "encrypted_value", "encryption_version"}))
broadcaster := &captureBroadcaster{}
registry := &mockEnvMutator{returnErr: errInternalDB}
handler := &WorkspaceHandler{
broadcaster: broadcaster,
cpProv: &provisioner.CPProvisioner{},
platformURL: "http://platform.test",
envMutators: registry,
}
handler.provisionWorkspaceCP("ws-cp-test-456", "", nil, models.CreateWorkspacePayload{Name: "test-cp"})
if broadcaster.lastData == nil {
t.Fatal("expected WORKSPACE_PROVISION_FAILED broadcast, got none")
}
errVal, ok := broadcaster.lastData["error"]
if !ok {
t.Fatal(`broadcast missing "error" key`)
}
errStr, ok := errVal.(string)
if !ok {
t.Fatalf("broadcast error field is not a string: %T", errVal)
}
if errStr == errInternalDB.Error() {
t.Errorf("CP provisioner broadcast error contains raw err.Error() = %q", errStr)
}
if errStr != "provisioning failed" {
t.Errorf("expected error=%q, got %q", "provisioning failed", errStr)
}
}
// mockEnvMutator is a provisionhook.Registry stub that always returns a fixed error.
@ -1121,14 +1182,87 @@ func (m *mockEnvMutator) Run(_ context.Context, _ string, _ map[string]string) e
return m.returnErr
}
func (m *mockEnvMutator) Register(_ interface{}) {}
func (m *mockEnvMutator) Register(_ provisionhook.EnvMutator) {}
// TestResolveAndStage_NoInternalErrorsInHTTPErr asserts that resolveAndStage
// never puts err.Error() in HTTP error responses.
//
// TODO(#1366): PluginsHandler.sources is a concrete *plugins.Registry so the
// mockPluginsSources stub can't be injected. Skipping until a larger test
// refactor interface-types the dependency.
// never puts err.Error() in HTTP error responses. Tests plugin source
// parsing, resolver failures, and validation errors.
func TestResolveAndStage_NoInternalErrorsInHTTPErr(t *testing.T) {
t.Skip("blocked on issue #1366 — restore after PluginsHandler.sources is interface-typed")
testCases := []struct {
name string
source string
wantSafe bool // true = expect 4xx, false = expect nil
wantHTTPError bool // true = expect *httpErr from resolveAndStage
// knownUnsafe, if non-empty, is a substring that must NOT appear in
// the error body when wantHTTPError is true.
knownUnsafe string
}{
{
name: "empty source",
source: "",
wantHTTPError: true,
knownUnsafe: "pq:",
},
{
name: "valid source",
source: "github://owner/repo",
wantHTTPError: false,
knownUnsafe: "pq:",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
h := &PluginsHandler{
sources: &mockPluginsSources{schemes: []string{"github", "local"}},
}
_, err := h.resolveAndStage(context.Background(), installRequest{Source: tc.source})
if tc.wantHTTPError {
if err == nil {
t.Fatal("expected an error, got nil")
}
httpErr, ok := err.(*httpErr)
if !ok {
t.Fatalf("expected *httpErr, got %T", err)
}
// Verify the generic message is used (not a raw err.Error()).
if httpErr.Body == nil {
t.Fatal("httpErr.Body is nil")
}
errStr, ok := httpErr.Body["error"].(string)
if !ok {
t.Fatalf("body error field is not a string: %T", httpErr.Body["error"])
}
if tc.knownUnsafe != "" && strings.Contains(errStr, tc.knownUnsafe) {
t.Errorf("error body contains unsafe string %q: %q", tc.knownUnsafe, errStr)
}
} else {
if err != nil && tc.wantHTTPError {
t.Errorf("unexpected error: %v", err)
}
}
})
}
}
// mockPluginsSources implements plugins.SourceResolver for testing.
type mockPluginsSources struct {
schemes []string
}
func (m *mockPluginsSources) Schemes() []string { return m.schemes }
func (m *mockPluginsSources) Resolve(source plugins.Source) (plugins.SourceResolver, error) {
if source.Scheme == "github" {
return &mockResolver{}, nil
}
return nil, fmt.Errorf("unsupported scheme %q", source.Scheme)
}
type mockResolver struct{}
func (*mockResolver) Scheme() string { return "" }
func (*mockResolver) Fetch(ctx context.Context, spec, destDir string) (string, error) {
return "", nil
}