Skip to content

Conversation

@Ishirui
Copy link
Contributor

@Ishirui Ishirui commented Oct 27, 2025

Adds a simple method wrapping go build for use in dda build commands.

@Ishirui Ishirui force-pushed the pierrelouis.veyrenc/ACIX-1062-go-build-helper branch 3 times, most recently from 69b83fd to 44d3f4b Compare November 5, 2025 12:44
@Ishirui Ishirui marked this pull request as ready for review November 5, 2025 13:15
@Ishirui Ishirui requested a review from a team as a code owner November 5, 2025 13:15
@Ishirui Ishirui force-pushed the pierrelouis.veyrenc/ACIX-1062-go-build-helper branch from c73f2fc to 58a61e2 Compare November 5, 2025 13:16
@Ishirui Ishirui changed the title ACIX-1062: Create helpers for builds using GO ACIX-1062: Create helper function for go build Nov 5, 2025
…o build`

Also restructures a bit the tests directory for `go`
This way we can use the build cache if needed, which is useful for example in the `test_build_project` test which rebuilds more or less the same project twice.
…rting with `-`

This would cause a failure in `test_command_formation` if the entrypoint starts with `-`, as it would then be considered a flag instead of a parameter
Copilot AI review requested due to automatic review settings December 9, 2025 13:56
@Ishirui Ishirui force-pushed the pierrelouis.veyrenc/ACIX-1062-go-build-helper branch from 58a61e2 to 37748c8 Compare December 9, 2025 13:56
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 adds a new build() helper method to the Go tool wrapper for running instrumented go build commands with various compilation flags. The implementation includes comprehensive test coverage with both unit tests for command formation and integration tests using a small Go project fixture.

Key Changes:

  • Added Go.build() method with support for build tags, compiler/linker flags, and platform-specific race detection
  • Reorganized test structure by moving tests/tools/test_go.py to tests/tools/go/test_go.py
  • Added test fixtures including a small Go project with build tag support
  • Enhanced test infrastructure with new pytest markers for skipping tests on specific platforms

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/dda/tools/go.py Adds build() and _build() methods for wrapping go build with automatic flag management and race detection
tests/tools/go/test_go.py Moved and expanded tests with parametrized test cases for command formation and integration tests
tests/tools/go/conftest.py Adds get_random_filename fixture for generating test file paths
tests/tools/go/__init__.py New package initialization file
tests/tools/go/fixtures/small_go_project/* Test fixture Go project with build tags for integration testing
tests/conftest.py Adds skip_* pytest markers for platform-specific test skipping and fixes requires_unix condition

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

Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

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

Nice!

- Additionnal quoting around some arguments
- Pass `tags` as a comma-separated list
- Pass custom env vars to the build command
- `-mod` argument has a different purpose than previously thought
@Ishirui Ishirui merged commit fd65f14 into main Dec 12, 2025
20 checks passed
@Ishirui Ishirui deleted the pierrelouis.veyrenc/ACIX-1062-go-build-helper branch December 12, 2025 08:54
github-actions bot pushed a commit that referenced this pull request Dec 12, 2025
* feat(go): Add a custom `Go.build` method for easier invocations of `go build`

Also restructures a bit the tests directory for `go`

* fix(go): Make the `-a` argument controllable

This way we can use the build cache if needed, which is useful for example in the `test_build_project` test which rebuilds more or less the same project twice.

* fix(build): Prevent `get_random_filename` from creating filenames starting with `-`

This would cause a failure in `test_command_formation` if the entrypoint starts with `-`, as it would then be considered a flag instead of a parameter

* feat(tests): Add `skip_${OS}` pytest markers for skipping tests on specific operating systems

* fix(tests): Skip `TestBuild::test_build_project` on macOS as `go` is not installed in CI runners

* fix(go): Tweak `Go.build` method interface after testing

- Additionnal quoting around some arguments
- Pass `tags` as a comma-separated list
- Pass custom env vars to the build command
- `-mod` argument has a different purpose than previously thought

* refactor: Minor Copilot-suggested tweaks

* refactor: Simplify `get_random_filename` test fixture

* refactor(go): Pass `*packages` instead of a single `entrypoint`

* refactor(tests): Use sets of build tags everywhere

* feat(go): Allow passing 0 packages to `build` method

* refactor(tests): `get_random_filename` returns a simple `str` fd65f14
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