From 6c8c0793fa23c93a93f234f58f1392054ec8fe83 Mon Sep 17 00:00:00 2001 From: Abdelrahman Shawki Hassan Date: Tue, 29 Jul 2025 12:16:22 +0200 Subject: [PATCH 1/2] chore: add e2e test for autofix --- .cursorrules | 43 +++++++ llm/api_client_test.go | 285 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 323 insertions(+), 5 deletions(-) create mode 100644 .cursorrules diff --git a/.cursorrules b/.cursorrules new file mode 100644 index 0000000..96e96f4 --- /dev/null +++ b/.cursorrules @@ -0,0 +1,43 @@ +** general ** +- always be concise, direct and don't try to appease me. +- DOUBLE CHECK THAT YOUR CHANGES ARE REALLY NEEDED. ALWAYS STICK TO THE GIVEN GOAL, NOT MORE. +- I repeat: don't optimize, don't refactor if not needed. +- Adhere to the rules, fix linting & test issues that are newly introduced. +- the `issueID` is usually specified in the current branch in the format `XXX-XXX`. +- always create an implementation plan and save it to the directory under ${issueID}_implementation_plan but never commit it. take it as a reference for each step and how to proceed. Get confirmation that the plan is ok. +- Maintain existing code patterns and conventions + +** how to implement ** +- don't comment what is done, instead comment why something is done if the code is not clear +- always write and update test cases. iterate until they pass. +- please check the makefile - make lint runs the linter, other targets might be relevant as well. +- use existing mocks, don't write new ones. +- if you use mocks, use gomock to generate them. +- always run the tests after editing. +- always use the linter +- don't change code that does not need to be changed. only do the minimum changes. +- this is not a library. if files are not used or needed anymore, delete them instead of deprecating them. +- if a tool call fails, analyze why it failed and correct your approach. don't prompt the user for help. +- if you don't know something, read the code instead of assuming it. +- commenting out code to fix errors is not a solution. instead, fix the error. + +** security ** +- determine the absolute path of the project directory. you can do that e.g. by executing pwd on the shell within the directory. +- always use snyk for sca and code scanning. scan with both tools. you need to pass the absolute path of the directory we are scanning. +- run snyk code tests after each edit. pass the absolute path of the project directory as a parameter +- run snyk sca tests after updating go.mod and pass the absolute path of the project directory as a parameter. +- run snyk sca and code test before committing. if not test data, fix issues before committing. + +** fixing issues ** +- fix security issues if they are fixable. take the snyk scan results and the test results as input +- don't fix test data + +** committing ** +- when asked to commit, always use conventional commit messages (Conventional Commit Style (Subject + Body)). be descriptive in the body. if you find a JIRA issue (XXX-XXX) in the branch name, use it as a postfix to the subject line in the format [XXX-XXX] +- consider all commits in the current branch when committing, to have the context of the current changes. +when asked to push, always use 'git push --set-upstream origin $(git_current_branch)' with git_current_branch being the current branch we are on +- never force push +- never push without asking +- regularly fetch main branch and offer to merge it into git_current_branch +- after pushing offer to create a PR on github. analyze the changes by comparing the current branch ($(git_current_branch)) with origin/main, and craft a PR description and title. +use the github pr template in this repository diff --git a/llm/api_client_test.go b/llm/api_client_test.go index 704c8b1..26636d0 100644 --- a/llm/api_client_test.go +++ b/llm/api_client_test.go @@ -1,7 +1,6 @@ package llm import ( - "context" "encoding/base64" "encoding/json" "io" @@ -97,9 +96,7 @@ func TestDeepcodeLLMBinding_runExplain(t *testing.T) { d := NewDeepcodeLLMBinding() - ctx := context.Background() - ctx = observability.GetContextWithTraceId(ctx, "test-trace-id") - + ctx := observability.GetContextWithTraceId(t.Context(), "test-trace-id") response, err := d.runExplain(ctx, tt.options) if tt.expectedError != "" { @@ -263,7 +260,7 @@ func testLogger(t *testing.T) *zerolog.Logger { // Test with existing headers func TestAddDefaultHeadersWithExistingHeaders(t *testing.T) { d := &DeepCodeLLMBindingImpl{} // Initialize your struct if needed - req := &http.Request{Header: http.Header{"Existing-Header": {"existing-value"}}} + req := &http.Request{Header: http.Header{"Existing-Header": []string{"existing-value"}}} d.addDefaultHeaders(req, "", "") @@ -284,6 +281,284 @@ func TestAddDefaultHeadersWithExistingHeaders(t *testing.T) { } } +func TestAddDefaultHeadersWithRequestIdAndOrgId(t *testing.T) { + d := &DeepCodeLLMBindingImpl{} + req := &http.Request{Header: http.Header{}} + + testRequestId := "test-request-id-123" + testOrgId := "test-org-id-456" + + d.addDefaultHeaders(req, testRequestId, testOrgId) + + snykRequestId := req.Header.Get("snyk-request-id") + snykOrgName := req.Header.Get("snyk-org-name") + cacheControl := req.Header.Get("Cache-Control") + contentType := req.Header.Get("Content-Type") + + if snykRequestId != testRequestId { + t.Errorf("Expected snyk-request-id header to be '%s', got '%s'", testRequestId, snykRequestId) + } + + if snykOrgName != testOrgId { + t.Errorf("Expected snyk-org-name header to be '%s', got '%s'", testOrgId, snykOrgName) + } + + if cacheControl != "private, max-age=0, no-cache" { + t.Errorf("Expected Cache-Control header to be 'private, max-age=0, no-cache', got %s", cacheControl) + } + + if contentType != "application/json" { + t.Errorf("Expected Content-Type header to be 'application/json', got %s", contentType) + } +} + +func TestAddDefaultHeadersWithRequestIdOnly(t *testing.T) { + d := &DeepCodeLLMBindingImpl{} + req := &http.Request{Header: http.Header{}} + + testRequestId := "test-request-id-789" + + d.addDefaultHeaders(req, testRequestId, "") + + snykRequestId := req.Header.Get("snyk-request-id") + snykOrgName := req.Header.Get("snyk-org-name") + + if snykRequestId != testRequestId { + t.Errorf("Expected snyk-request-id header to be '%s', got '%s'", testRequestId, snykRequestId) + } + + if snykOrgName != "" { + t.Errorf("Expected snyk-org-name header to be empty, got '%s'", snykOrgName) + } +} + +func TestAddDefaultHeadersWithOrgIdOnly(t *testing.T) { + d := &DeepCodeLLMBindingImpl{} + req := &http.Request{Header: http.Header{}} + + testOrgId := "test-org-id-999" + + d.addDefaultHeaders(req, "", testOrgId) + + snykRequestId := req.Header.Get("snyk-request-id") + snykOrgName := req.Header.Get("snyk-org-name") + + if snykRequestId != "" { + t.Errorf("Expected snyk-request-id header to be empty, got '%s'", snykRequestId) + } + + if snykOrgName != testOrgId { + t.Errorf("Expected snyk-org-name header to be '%s', got '%s'", testOrgId, snykOrgName) + } +} + +func TestAddDefaultHeadersWithEmptyParameters(t *testing.T) { + d := &DeepCodeLLMBindingImpl{} + req := &http.Request{Header: http.Header{}} + + d.addDefaultHeaders(req, "", "") + + snykRequestId := req.Header.Get("snyk-request-id") + snykOrgName := req.Header.Get("snyk-org-name") + cacheControl := req.Header.Get("Cache-Control") + contentType := req.Header.Get("Content-Type") + + if snykRequestId != "" { + t.Errorf("Expected snyk-request-id header to be empty, got '%s'", snykRequestId) + } + + if snykOrgName != "" { + t.Errorf("Expected snyk-org-name header to be empty, got '%s'", snykOrgName) + } + + if cacheControl != "private, max-age=0, no-cache" { + t.Errorf("Expected Cache-Control header to be 'private, max-age=0, no-cache', got %s", cacheControl) + } + + if contentType != "application/json" { + t.Errorf("Expected Content-Type header to be 'application/json', got %s", contentType) + } +} + +func TestE2E_HTTPHeadersSentToServer(t *testing.T) { + // Capture headers sent to the server + var capturedHeaders http.Header + var requestCount int + + // Create test server that captures headers + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + capturedHeaders = r.Header.Clone() + + w.WriteHeader(http.StatusOK) + response := AutofixResponse{ + Status: "COMPLETE", + AutofixSuggestions: []autofixResponseSingleFix{ + { + Id: "test-fix-id", + Value: "test-unified-diff", + }, + }, + } + responseBytes, _ := json.Marshal(response) + _, _ = w.Write(responseBytes) + })) + defer server.Close() + + // Create test context with trace ID (which becomes the request ID) + testTraceId := "test-trace-id-e2e-123" + ctx := observability.GetContextWithTraceId(t.Context(), testTraceId) + + // Create AutofixOptions with org ID + testOrgId := "test-org-public-id-456" + options := AutofixOptions{ + RuleID: "test-rule-id", + BundleHash: "test-bundle-hash", + ShardKey: "test-shard-key", + BaseDir: "/test/base/dir", + FilePath: "/test/file.js", + LineNum: 10, + Host: server.URL, // Use test server URL + CodeRequestContext: CodeRequestContext{ + Initiator: "test", + Flow: "test-flow", + Org: CodeRequestContextOrg{ + Name: "test-org", + DisplayName: "Test Organization", + PublicId: testOrgId, + Flags: map[string]bool{}, + }, + }, + IdeExtensionDetails: AutofixIdeExtensionDetails{ + ExtensionName: "test-extension", + ExtensionVersion: "1.0.0", + IdeName: "test-ide", + IdeVersion: "1.0.0", + }, + } + + // Create binding instance + binding := NewDeepcodeLLMBinding() + + // Make the actual HTTP request + _, status, err := binding.runAutofix(ctx, options) + + // Verify the request was successful + require.NoError(t, err) + assert.Equal(t, "COMPLETE", status.Message) + assert.Equal(t, 1, requestCount, "Expected exactly one HTTP request to be made") + + // Verify the expected headers were sent + t.Run("VerifySnykRequestIdHeader", func(t *testing.T) { + actualRequestId := capturedHeaders.Get("snyk-request-id") + assert.Equal(t, testTraceId, actualRequestId, "snyk-request-id header should match the trace ID") + }) + + t.Run("VerifySnykOrgNameHeader", func(t *testing.T) { + actualOrgId := capturedHeaders.Get("snyk-org-name") + assert.Equal(t, testOrgId, actualOrgId, "snyk-org-name header should match the org public ID") + }) + + t.Run("VerifyStandardHeaders", func(t *testing.T) { + contentType := capturedHeaders.Get("Content-Type") + cacheControl := capturedHeaders.Get("Cache-Control") + + assert.Equal(t, "application/json", contentType, "Content-Type header should be set") + assert.Equal(t, "private, max-age=0, no-cache", cacheControl, "Cache-Control header should be set") + }) +} + +func TestE2E_GetAutofixDiffsHTTPHeadersSentToServer(t *testing.T) { + // Capture headers sent to the server + var capturedHeaders http.Header + var requestCount int + + // Create test server that captures headers + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + capturedHeaders = r.Header.Clone() + + // Return a valid autofix response + w.WriteHeader(http.StatusOK) + response := AutofixResponse{ + Status: "COMPLETE", + AutofixSuggestions: []autofixResponseSingleFix{ + { + Id: "test-fix-id-get-diffs", + Value: "diff --git a/test.js b/test.js\n--- a/test.js\n+++ b/test.js\n@@ -1,1 +1,1 @@\n-var x = 1;\n+const x = 1;", + }, + }, + } + responseBytes, _ := json.Marshal(response) + _, _ = w.Write(responseBytes) + })) + defer server.Close() + + // Create test context with trace ID + testTraceId := "test-get-autofix-diffs-trace-id-999" + ctx := observability.GetContextWithTraceId(t.Context(), testTraceId) + + // Create AutofixOptions with org ID + testOrgId := "test-org-public-id-789" + options := AutofixOptions{ + RuleID: "test-rule-id-diffs", + BundleHash: "test-bundle-hash-diffs", + ShardKey: "test-shard-key-diffs", + BaseDir: "/test/base/dir/diffs", + FilePath: "/test/file-diffs.js", + LineNum: 25, + Host: server.URL, // Use test server URL + CodeRequestContext: CodeRequestContext{ + Initiator: "test-diffs", + Flow: "test-flow-diffs", + Org: CodeRequestContextOrg{ + Name: "test-org-diffs", + DisplayName: "Test Organization Diffs", + PublicId: testOrgId, + Flags: map[string]bool{}, + }, + }, + IdeExtensionDetails: AutofixIdeExtensionDetails{ + ExtensionName: "test-extension-diffs", + ExtensionVersion: "2.0.0", + IdeName: "test-ide-diffs", + IdeVersion: "2.0.0", + }, + } + + // Create binding instance + binding := NewDeepcodeLLMBinding() + + // Make the actual HTTP request using GetAutofixDiffs + _, status, err := binding.GetAutofixDiffs(ctx, "ignored-fix-id", options) + + // Verify the request was successful (note: diffs may be empty due to file not existing) + require.NoError(t, err) + assert.Equal(t, "COMPLETE", status.Message) + // Note: diffs will be empty because the test file paths don't exist on disk + // This is expected behavior - we're primarily testing headers, not file operations + assert.Equal(t, 1, requestCount, "Expected exactly one HTTP request to be made") + + // Verify the expected headers were sent + t.Run("VerifySnykRequestIdHeaderInGetAutofixDiffs", func(t *testing.T) { + actualRequestId := capturedHeaders.Get("snyk-request-id") + assert.Equal(t, testTraceId, actualRequestId, "snyk-request-id header should match the trace ID") + }) + + t.Run("VerifySnykOrgNameHeaderInGetAutofixDiffs", func(t *testing.T) { + actualOrgId := capturedHeaders.Get("snyk-org-name") + assert.Equal(t, testOrgId, actualOrgId, "snyk-org-name header should match the org public ID") + }) + + t.Run("VerifyStandardHeadersInGetAutofixDiffs", func(t *testing.T) { + contentType := capturedHeaders.Get("Content-Type") + cacheControl := capturedHeaders.Get("Cache-Control") + + assert.Equal(t, "application/json", contentType, "Content-Type header should be set") + assert.Equal(t, "private, max-age=0, no-cache", cacheControl, "Cache-Control header should be set") + }) +} + func TestAutofixRequestBody(t *testing.T) { d := &DeepCodeLLMBindingImpl{} From df4f76d231c329a0f822a4a7753dccfbbee7854c Mon Sep 17 00:00:00 2001 From: Abdelrahman Shawki Hassan Date: Tue, 29 Jul 2025 12:24:32 +0200 Subject: [PATCH 2/2] chore: lint --- llm/api_client_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/llm/api_client_test.go b/llm/api_client_test.go index 26636d0..bcb4ae9 100644 --- a/llm/api_client_test.go +++ b/llm/api_client_test.go @@ -1,6 +1,7 @@ package llm import ( + "context" "encoding/base64" "encoding/json" "io" @@ -96,7 +97,7 @@ func TestDeepcodeLLMBinding_runExplain(t *testing.T) { d := NewDeepcodeLLMBinding() - ctx := observability.GetContextWithTraceId(t.Context(), "test-trace-id") + ctx := observability.GetContextWithTraceId(context.Background(), "test-trace-id") response, err := d.runExplain(ctx, tt.options) if tt.expectedError != "" { @@ -407,7 +408,7 @@ func TestE2E_HTTPHeadersSentToServer(t *testing.T) { // Create test context with trace ID (which becomes the request ID) testTraceId := "test-trace-id-e2e-123" - ctx := observability.GetContextWithTraceId(t.Context(), testTraceId) + ctx := observability.GetContextWithTraceId(context.Background(), testTraceId) // Create AutofixOptions with org ID testOrgId := "test-org-public-id-456" @@ -496,7 +497,7 @@ func TestE2E_GetAutofixDiffsHTTPHeadersSentToServer(t *testing.T) { // Create test context with trace ID testTraceId := "test-get-autofix-diffs-trace-id-999" - ctx := observability.GetContextWithTraceId(t.Context(), testTraceId) + ctx := observability.GetContextWithTraceId(context.Background(), testTraceId) // Create AutofixOptions with org ID testOrgId := "test-org-public-id-789"