-
Notifications
You must be signed in to change notification settings - Fork 10
Add POST request tests to validate WARC capture of request bodies #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Add TestHTTPClientPOSTWithTextPayload to test POST with plain text - Add TestHTTPClientPOSTWithJSONPayload to test POST with JSON data - Add TestHTTPClientPOSTWithFormData to test POST with form-encoded data - All tests verify request body is captured correctly in WARC files - Add missing imports (strconv, net/url) for new test functions Co-authored-by: CorentinB <[email protected]>
NGTmeaty
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please make sure you rebase against master branch and please look at other tests for a general sense of the style. thank you.
client_test.go
Outdated
|
|
||
| // Check the WARC records contain the POST request with form data | ||
| for _, path := range files { | ||
| testFileHash(t, path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use testFileSingleHashCheck on this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated TestHTTPClientPOSTWithFormData to use testFileSingleHashCheck as requested (commit 4474dcb).
Bumps the go-modules group with 2 updates: [github.com/klauspost/compress](https://github.com/klauspost/compress) and [github.com/refraction-networking/utls](https://github.com/refraction-networking/utls). Updates `github.com/klauspost/compress` from 1.18.0 to 1.18.1 - [Release notes](https://github.com/klauspost/compress/releases) - [Changelog](https://github.com/klauspost/compress/blob/master/.goreleaser.yml) - [Commits](klauspost/compress@v1.18.0...v1.18.1) Updates `github.com/refraction-networking/utls` from 1.8.0 to 1.8.1 - [Release notes](https://github.com/refraction-networking/utls/releases) - [Commits](refraction-networking/utls@v1.8.0...v1.8.1) --- updated-dependencies: - dependency-name: github.com/klauspost/compress dependency-version: 1.18.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-modules - dependency-name: github.com/refraction-networking/utls dependency-version: 1.8.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-modules ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
The lifecycle of warcVer spans multiple `readUntilDelim()` calls, and changes in the underlying buffer (intermediateBuf) data can cause unexpected to the warcVer.
…150) * Add platform-specific memory management for macOS to spooledtempfile This commit adds macOS support to the spooledtempfile package's memory management functionality, which previously only supported Linux. Changes: - Split memory.go into platform-specific implementations using Go build tags - memory_linux.go: Linux implementation (cgroups v1/v2, /proc/meminfo) - memory_darwin.go: macOS implementation (sysctl VM statistics) - memory.go: Shared caching logic for all platforms - Added comprehensive platform-specific tests - memory_linux_test.go: Tests for cgroup and /proc/meminfo - memory_darwin_test.go: Tests for sysctl memory retrieval - memory_test.go: Platform-agnostic tests - Updated CI/CD workflow (.github/workflows/go.yml) - Added matrix strategy to test both ubuntu-latest and macos-latest - Platform-specific test verification for both Linux and macOS - Cross-compilation checks to ensure compatibility - Made spooled tests deterministic with memory mocking - Added mockMemoryUsage() helper for DRY test code - Fixed 8 tests that were failing on high-memory systems - Added 3 new threshold boundary tests (49%, 50%, 51%) - All tests now pass regardless of host system memory state The macOS implementation uses sysctl to query VM statistics and calculates memory usage as: (total - free - purgeable - speculative) / total All tests pass on both platforms with proper mocking ensuring deterministic behavior independent of actual system memory usage. * Fix test failure from cache state pollution between test packages Add ResetMemoryCache() function and call it in test cleanup to prevent global memoryUsageCache state from persisting across test packages. When running 'go test ./...', tests from pkg/spooledtempfile that mock memory usage (mocking high memory values like 60%) would pollute the global cache, causing TestHTTPClientRequestFailing to fail when run after them. The fix adds: 1. ResetMemoryCache() - clears the global cache (lastChecked and lastFraction) 2. Calls ResetMemoryCache() in mockMemoryUsage() cleanup to ensure clean state This maintains the performance benefits of the global cache while ensuring test isolation through explicit cleanup using t.Cleanup(). * Fix PR #150 review feedback: uint64 underflow, hardcoded paths, and endianness Address three issues identified in code review: 1. **Fix potential uint64 underflow** (memory_darwin.go): - If reclaimablePages > totalPages, subtraction would wrap to large number - Now clamps usedPages to 0 when reclaimable pages exceed total - Adds explicit bounds checking to prevent invalid memory fractions 2. **Fix hardcoded error path** (memory_linux.go): - Changed error message from literal "/proc/meminfo" to use procMeminfoPath variable - Improves diagnostic accuracy when paths are overridden in tests 3. **Simplify endianness handling** (memory_darwin.go): - Removed custom getSysctlUint32() helper function - Now uses unix.SysctlUint32() directly which handles endianness correctly - Removed unnecessary encoding/binary import - Updated tests to use unix.SysctlUint32() directly All tests pass. Cross-compilation verified for both Linux and macOS. * Fix testdata path resolution in mend tests to work from any directory Replace hardcoded relative paths with dynamic path resolution using runtime.Caller() to compute the testdata directory relative to the test file location. This fixes tests being skipped in CI/CD when running 'go test ./...' from the root directory. Tests now correctly find and run against testdata files. Changes: - Added getTestdataDir() helper function using runtime.Caller() - Replaced 5 hardcoded "../../testdata/warcs" paths with getTestdataDir() - Added runtime package import - Tests now run from any working directory (root, CI/CD, etc.) All TestAnalyzeWARCFile subtests now run and pass instead of being skipped. * Fix TestMendFunctionDirect path resolution to work in CI/CD Replace os.Getwd() + relative path construction with getTestdataDir() helper to make TestMendFunctionDirect work from any directory. This fixes 4 skipped subtests: - good.warc.gz.open - empty.warc.gz.open - corrupted-trailing-bytes.warc.gz.open - corrupted-mid-record.warc.gz.open These tests now run properly in CI/CD when 'go test ./...' is run from root. * Fix TestHTTPClient byte range tolerance for macOS compatibility Widen the acceptable byte range in TestHTTPClient from 27130-27160 to 27130-27170 to accommodate platform-specific differences in HTTP response headers between macOS and Linux. The test was failing on macOS with 27163 bytes due to platform-specific header variations, which is normal behavior across different operating systems. * Fix WARC version parsing and mend expectations * Extend CI job timeout to 15 minutes * Make HTTP client tests deterministic * Address copilot feedback on PR #150 - Remove unused vm.page_pageable_internal/external_count sysctls that cause failures on some macOS versions - Fix data races in memory_test.go by using ResetMemoryCache() and proper mutex locking - Fix cache pointer reassignment in spooled_test.go to prevent race conditions - Update CI test filter to reference only existing tests (TestMemoryFractionConsistency instead of TestGetSysctlUint32) * Limit macOS CI to spooledtempfile tests only Run only spooledtempfile package tests on macOS runners instead of the full test suite, since the macOS-specific code changes are limited to that package. This addresses review feedback to optimize CI runtime while maintaining platform-specific test coverage. * Add WARC format regression smoke test Introduce TestSmokeWARCFormatRegression to validate WARC format consistency using a frozen reference file (testdata/test.warc.gz). This test checks exact byte counts, record counts, Content-Length values, and digest hashes against known-good values. This complements the existing dynamic tests by providing explicit validation that the WARC format hasn't changed, addressing the concern about byte-level format regression detection while keeping the main integration tests maintainable.
…tes (#158) Bumps the go-modules group with 2 updates in the / directory: [golang.org/x/net](https://github.com/golang/net) and [golang.org/x/sync](https://github.com/golang/sync). Updates `golang.org/x/net` from 0.46.0 to 0.47.0 - [Commits](golang/net@v0.46.0...v0.47.0) Updates `golang.org/x/sync` from 0.17.0 to 0.18.0 - [Commits](golang/sync@v0.17.0...v0.18.0) Updates `golang.org/x/sys` from 0.37.0 to 0.38.0 - [Commits](golang/sys@v0.37.0...v0.38.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-version: 0.47.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-modules - dependency-name: golang.org/x/sync dependency-version: 0.18.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-modules - dependency-name: golang.org/x/sys dependency-version: 0.38.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-modules ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: add windows memory I have tested locally and I'm unaware of a way to do this without `unsafe` as the native Go libraries do not support it. * fix: remove redundant tests
Co-authored-by: NGTmeaty <[email protected]>
Rebased against master branch (a5b6f10) and updated all three POST tests to use testFileSingleHashCheck following the existing test patterns (commit 4474dcb). Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Plan to Address Review Comments
Changes Made
Rebased on Master
Updated All POST Tests to Use testFileSingleHashCheck
Replaced custom WARC file validation with
testFileSingleHashCheckin all three POST tests:TestHTTPClientPOSTWithTextPayload
testFileSingleHashCheckwith hashsha1:RFV2ZU2BHITF3PW7BSPBQE65GFZS7F5GTestHTTPClientPOSTWithJSONPayload
testFileSingleHashCheckwith hashsha1:IAKLOIOTQX2W7PAAWWA2TELLU5HCKO3VTestHTTPClientPOSTWithFormData
testFileSingleHashCheckwith hashsha1:DGXE2J6TLUT3GYLTA2LNA4NQMMPF5SWXTest Results
go fmtgo vet(no issues)Original prompt
Fixes #122
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.