-
Notifications
You must be signed in to change notification settings - Fork 0
fix(testing): remove codspeed folder used for integration tests #45
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
fix(testing): remove codspeed folder used for integration tests #45
Conversation
36a2c6f to
13695f1
Compare
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.
Pull request overview
This PR introduces a utility function to remove the "codspeed" folder from file paths in the testing framework, improving the user experience by cleaning up file paths shown in test output and benchmark metadata. The change is part of handling integration tests that use codspeed benchmark files.
Key Changes:
- Added
removeFolderFromPath()helper function to strip "codspeed" folder from file paths - Applied folder removal in two contexts: callsite reporting and benchmark file tracking
- Imported
stringspackage to support the new functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| testing/testing/codspeed.go | Added new removeFolderFromPath function that strips specified folder names from paths by splitting on path separator |
| testing/testing/testing.go | Applied folder removal to callsite file paths when handling _codspeed.go files |
| testing/testing/benchmark.go | Applied folder removal to benchmark file paths when processing codspeed benchmark results |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #45 will improve performances by 21.83%Comparing Summary
Benchmarks breakdown
|
f3421ba to
f042bfc
Compare
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.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...r/src/snapshots/codspeed_go_runner__integration_tests__assert_results_snapshots@example.snap
Outdated
Show resolved
Hide resolved
...r/src/snapshots/codspeed_go_runner__integration_tests__assert_results_snapshots@example.snap
Outdated
Show resolved
Hide resolved
f042bfc to
f3513dc
Compare
adriencaccia
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.
LGTM, let's just make the benchmarks more relevant
f3513dc to
bbc9ccc
Compare
Fixes #44
Since we're modifying the code for our custom Go runner, we need to properly fix the paths used in the URI. We already replaced
_codspeed.gowith_test.go, but we also have to remove thecodspeed/folder which is used for integration tests.