Skip to content

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Nov 7, 2025

Forgot to properly patch the imports of the testing fork, so we had an error when we tried to include the testing/synctest package because it was still using the upstream testing package.

It's now fixed, and I added a few tests to ensure we catch this in the future.

EDIT: Also fixed a build error in fuego that was caused by them using workspaces. We're now panicking in tests when the build fails to catch those errors earlier

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes import issues in the testing package fork, specifically addressing problems with the testing/synctest package that was still referencing the upstream testing package instead of the CodSpeed fork. The fix includes patching imports across multiple files and adding tests to prevent similar issues in the future.

  • Updated import statements throughout the testing fork to use the CodSpeedHQ fork path
  • Renamed linkname functions to avoid linker blocklists
  • Added integration tests with new test files to verify synctest functionality

Reviewed Changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated no comments.

Show a summary per file
File Description
testing/testing/testing.go Updated import examples and renamed linkname function to avoid blocklist
testing/testing/synctest/synctest.go Fixed imports and added documentation for renamed linkname function
testing/testing/slogtest/slogtest.go Updated testing import to use forked package
testing/patches/synctest.patch Added new patch file for synctest modifications
testing/internal/testenv/testenv.go Fixed testing import path
testing/internal/testenv/exec.go Fixed testing import path
testing/internal/synctest/synctest.go Added manual Escape implementation to avoid abi dependency
testing/internal/abi/*.go Removed entire abi package and related files
testing/fork.sh Updated script to remove abi from copied packages and apply synctest patch
go-runner/src/snapshots/*.snap Updated test snapshots with new benchmark entries
go-runner/src/lib.rs Added panic on build failure in test mode for early detection
go-runner/src/builder/mod.rs Disabled workspace mode to avoid build flag conflicts
example/sub-packages/buffer_test.go Added new test file with synctest usage examples
example/sub-packages/buffer.go Added supporting code for new tests
Comments suppressed due to low confidence (4)

testing/patches/synctest.patch:1

  • Corrected spelling of 'lead' to 'led' for past tense consistency.
    testing/patches/synctest.patch:1
  • Corrected spelling of 'linked' to 'linker'.
    testing/testing/synctest/synctest.go:1
  • Corrected spelling of 'linked' to 'linker'.
    testing/internal/synctest/synctest.go:1
  • Corrected spelling of 'lead' to 'led' for past tense consistency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 7, 2025

CodSpeed Performance Report

Merging #30 will improve performances by 15.38%

Comparing fix-testing-imports (d8d04cb) with main (bd7a35b)

Summary

⚡ 1 improvement
✅ 23 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
BenchmarkLargeSetupInLoop 30 ns 26 ns +15.38%

Copy link

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

olgtm

@not-matthias not-matthias merged commit d8d04cb into main Nov 7, 2025
15 checks passed
@not-matthias not-matthias deleted the fix-testing-imports branch November 7, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants