Skip to content

Commit 7cc9924

Browse files
committed
On branch main
modified: internal/browser/analysis_context.go modified: internal/browser/analysis_context_test.go modified: internal/browser/browser_helper_test.go modified: internal/browser/harvester.go modified: internal/browser/interactor.go modified: internal/browser/interactor_test.go modified: internal/browser/interface.go modified: internal/browser/manager.go modified: internal/browser/manager_test.go modified: internal/config/config.go modified: internal/observability/logger.go
1 parent 9a6eacc commit 7cc9924

15 files changed

Lines changed: 1217 additions & 908 deletions

internal/browser/analysis_context.go

Lines changed: 450 additions & 231 deletions
Large diffs are not rendered by default.

internal/browser/analysis_context_test.go

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// analysis_context_test.go
21
package browser_test
32

43
import (
@@ -16,13 +15,12 @@ import (
1615
)
1716

1817
func TestAnalysisContext_InitializeAndClose(t *testing.T) {
19-
t.Parallel()
2018
fixture, cleanup := newTestFixture(t)
19+
t.Parallel()
2120
defer cleanup()
2221

2322
server := createStaticTestServer(t, `<!DOCTYPE html><html><body><h1>Init Test</h1></body></html>`)
2423

25-
// The session is now directly in the fixture.
2624
session := fixture.Session
2725
assert.NotEmpty(t, session.ID(), "AnalysisContext should have a valid ID")
2826

@@ -36,8 +34,8 @@ func TestAnalysisContext_InitializeAndClose(t *testing.T) {
3634
}
3735

3836
func TestAnalysisContext_NavigateAndCollectArtifacts(t *testing.T) {
39-
t.Parallel()
4037
fixture, cleanup := newTestFixture(t)
38+
t.Parallel()
4139
defer cleanup()
4240

4341
server := createTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -52,9 +50,9 @@ func TestAnalysisContext_NavigateAndCollectArtifacts(t *testing.T) {
5250
sessionStorage.setItem('tempData', 'active');
5351
console.info('Initializing application...');
5452
fetch('/api/data')
55-
.then(res => res.text())
56-
.then(data => console.log('API data fetched:', data.length));
57-
setTimeout(() => { console.error("An expected error occurred."); }, 50);
53+
.then(res => res.text())
54+
.then(data => console.log('API data fetched:', data.length));
55+
setTimeout(() => { console.error("An expected error occurred."); }, 50);
5856
</script>
5957
</body>
6058
</html>
@@ -83,7 +81,6 @@ func TestAnalysisContext_NavigateAndCollectArtifacts(t *testing.T) {
8381
assert.Equal(t, "darkmode", storage.LocalStorage["userPref"], "LocalStorage mismatch")
8482
assert.Equal(t, "active", storage.SessionStorage["tempData"], "SessionStorage mismatch")
8583

86-
// Refactored cookie verification for more precise assertions.
8784
var sessionCookie *network.Cookie
8885
for _, cookie := range storage.Cookies {
8986
if cookie.Name == "SessionID" {
@@ -115,13 +112,13 @@ func TestAnalysisContext_NavigateAndCollectArtifacts(t *testing.T) {
115112
}
116113

117114
func TestSessionIsolation(t *testing.T) {
118-
t.Parallel()
119-
// For this specific test, we need two separate fixtures to get two separate sessions.
120115
fixture1, cleanup1 := newTestFixture(t)
116+
t.Parallel()
121117
defer cleanup1()
122118
session1 := fixture1.Session
123119

124120
fixture2, cleanup2 := newTestFixture(t)
121+
t.Parallel()
125122
defer cleanup2()
126123
session2 := fixture2.Session
127124

@@ -149,8 +146,8 @@ func TestSessionIsolation(t *testing.T) {
149146
}
150147

151148
func TestAnalysisContext_Interaction(t *testing.T) {
152-
t.Parallel()
153149
fixture, cleanup := newTestFixture(t)
150+
t.Parallel()
154151
defer cleanup()
155152
server := createStaticTestServer(t, `
156153
<!DOCTYPE html>
@@ -168,10 +165,10 @@ func TestAnalysisContext_Interaction(t *testing.T) {
168165
require.NoError(t, err)
169166

170167
interactionConfig := schemas.InteractionConfig{
171-
MaxDepth: 2,
168+
MaxDepth: 2,
172169
MaxInteractionsPerDepth: 5,
173-
InteractionDelayMs: 10,
174-
PostInteractionWaitMs: 100,
170+
InteractionDelayMs: 10,
171+
PostInteractionWaitMs: 100,
175172
}
176173

177174
err = session.Interact(interactionConfig)
@@ -197,7 +194,6 @@ func assertLogPresent(t *testing.T, logs []schemas.ConsoleLog, level, substring
197194
func findHAREntry(har *schemas.HAR, urlSubstring string) *schemas.Entry {
198195
for i, entry := range har.Log.Entries {
199196
if strings.Contains(entry.Request.URL, urlSubstring) {
200-
// Return a pointer to the actual entry in the slice.
201197
return &har.Log.Entries[i]
202198
}
203199
}

internal/browser/browser_helper_test.go

Lines changed: 64 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// browser_helper_test.go
21
package browser_test
32

43
import (
@@ -10,7 +9,6 @@ import (
109
"testing"
1110
"time"
1211

13-
"github.com/stretchr/testify/require"
1412
"go.uber.org/zap"
1513

1614
"github.com/xkilldash9x/scalpel-cli/internal/browser"
@@ -20,37 +18,63 @@ import (
2018
)
2119

2220
var (
23-
sharedManager *browser.Manager
24-
testLogger *zap.Logger
21+
testLogger *zap.Logger
22+
testManager *browser.Manager
23+
testConfig *config.Config
2524
)
2625

27-
// testFixture holds all the necessary components for a single, isolated browser test.
26+
// testFixture holds components for a single, isolated browser test.
2827
type testFixture struct {
2928
Session *browser.AnalysisContext
3029
Config *config.Config
3130
}
3231

33-
// TestMain sets up the shared browser manager for all tests in the package
34-
// and guarantees its shutdown after all tests have completed.
32+
// TestMain sets up a SINGLE, shared browser manager for all tests (Principle 1).
3533
func TestMain(m *testing.M) {
3634
var err error
3735
testLogger = getTestLogger()
38-
// Use a background context as the manager's lifecycle is for the whole package.
39-
sharedManager, err = browser.NewManager(context.Background(), testLogger, 4)
36+
37+
const testConcurrency = 4
38+
39+
// Configuration for the tests, matching the updated config.BrowserConfig.
40+
browserCfg := config.BrowserConfig{
41+
Headless: true,
42+
DisableCache: true,
43+
Concurrency: testConcurrency,
44+
Humanoid: humanoid.DefaultConfig(),
45+
Debug: false, // Set to true for verbose CDP logs (Principle 5).
46+
}
47+
48+
testConfig = &config.Config{
49+
Browser: browserCfg,
50+
Network: config.NetworkConfig{
51+
CaptureResponseBodies: true,
52+
NavigationTimeout: 30 * time.Second, // Matching the updated config.NetworkConfig.
53+
// PostLoadWait is deprecated in the refactored code (Principle 2).
54+
},
55+
}
56+
57+
// Create the manager once. Use context.Background() as the initCtx for the test suite lifetime.
58+
// Principle 3: Timeout for initialization.
59+
initCtx, cancel := context.WithTimeout(context.Background(), 45*time.Second)
60+
defer cancel()
61+
62+
// Call the updated NewManager function signature.
63+
testManager, err = browser.NewManager(initCtx, testLogger, browserCfg)
4064
if err != nil {
41-
testLogger.Fatal("Failed to create shared browser manager for tests", zap.Error(err))
65+
testLogger.Fatal("Failed to initialize browser manager for test suite", zap.Error(err))
4266
}
4367

44-
// m.Run() executes all tests in the package.
68+
// Run all the tests.
4569
code := m.Run()
4670

47-
// This block runs AFTER all tests are finished, including parallel ones.
48-
testLogger.Info("Shutting down shared browser manager after tests.")
49-
shutdownCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
50-
defer cancel()
51-
if err := sharedManager.Shutdown(shutdownCtx); err != nil {
52-
testLogger.Error("Error during shared manager shutdown", zap.Error(err))
71+
// Shut down the manager (Principle 4).
72+
shutdownCtx, cancelShutdown := context.WithTimeout(context.Background(), 15*time.Second)
73+
defer cancelShutdown()
74+
if err := testManager.Shutdown(shutdownCtx); err != nil {
75+
testLogger.Error("Error during test manager shutdown", zap.Error(err))
5376
}
77+
5478
os.Exit(code)
5579
}
5680

@@ -63,49 +87,51 @@ func getTestLogger() *zap.Logger {
6387
return logger
6488
}
6589

66-
// newTestFixture creates a fully initialized AnalysisContext for a test.
90+
// newTestFixture acquires a new session from the shared manager.
6791
func newTestFixture(t *testing.T) (*testFixture, func()) {
6892
t.Helper()
6993

70-
cfg := &config.Config{
71-
Browser: config.BrowserConfig{
72-
Headless: true,
73-
DisableCache: true,
74-
Humanoid: humanoid.DefaultConfig(),
75-
},
76-
Network: config.NetworkConfig{
77-
CaptureResponseBodies: true,
78-
PostLoadWait: 250 * time.Millisecond,
79-
},
80-
}
81-
persona := stealth.DefaultPersona
82-
testCtx, cancelTest := context.WithTimeout(context.Background(), 30*time.Second)
94+
// Principle 3: Timeout for acquiring and initializing the session.
95+
sessionCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
96+
97+
session, err := testManager.NewAnalysisContext(
98+
sessionCtx,
99+
testConfig,
100+
stealth.DefaultPersona,
101+
"", // No taint template
102+
"", // No taint config
103+
)
83104

84-
session, err := sharedManager.NewAnalysisContext(testCtx, cfg, persona, "", "")
85-
require.NoError(t, err, "Failed to initialize session from manager for test fixture")
105+
if err != nil {
106+
cancel()
107+
t.Fatalf("Failed to create new analysis session: %v", err)
108+
}
86109

87110
fixture := &testFixture{
88111
Session: session,
89-
Config: cfg,
112+
Config: testConfig,
90113
}
91114

92115
cleanup := func() {
93-
session.Close(context.Background())
94-
cancelTest()
116+
// Use a new context for cleanup in case the sessionCtx was cancelled (Principle 4).
117+
closeCtx, closeCancel := context.WithTimeout(context.Background(), 10*time.Second)
118+
defer closeCancel()
119+
session.Close(closeCtx)
120+
cancel()
95121
}
96122

97123
return fixture, cleanup
98124
}
99125

100-
// createTestServer creates an httptest.Server for dynamic content tests.
126+
// createTestServer creates an httptest.Server.
101127
func createTestServer(t *testing.T, handler http.Handler) *httptest.Server {
102128
t.Helper()
103129
server := httptest.NewServer(handler)
104130
t.Cleanup(server.Close)
105131
return server
106132
}
107133

108-
// createStaticTestServer creates an httptest.Server for serving simple, static HTML.
134+
// createStaticTestServer creates an httptest.Server for static HTML.
109135
func createStaticTestServer(t *testing.T, htmlContent string) *httptest.Server {
110136
t.Helper()
111137
return createTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)