Skip to content

chore: remove testonly for non-test targets#10137

Open
nmattia wants to merge 1 commit intomasterfrom
nm-remove-testonly
Open

chore: remove testonly for non-test targets#10137
nmattia wants to merge 1 commit intomasterfrom
nm-remove-testonly

Conversation

@nmattia
Copy link
Copy Markdown
Contributor

@nmattia nmattia commented May 8, 2026

The testonly tag was used very liberally throughout the codebase, and was applied to many targets that definitely were not test only (like ic-admin, published pocket-ic executables, etc). This removes some of the uses of testonly (it tends to spread like wildfire).

The `testonly` tag was used very liberally throughout the codebase, and
was applied to many targets that definitely were not test only (like
ic-admin, published pocket-ic executables, etc). This removes some of
the uses of `testonly` (it tends to spread like wildfire).
@github-actions github-actions Bot added the chore label May 8, 2026
@nmattia
Copy link
Copy Markdown
Contributor Author

nmattia commented May 8, 2026

(will need to update the cargo deps as well)

@nmattia nmattia marked this pull request as ready for review May 8, 2026 10:53
@nmattia nmattia requested review from a team as code owners May 8, 2026 10:53
github-actions[bot]

This comment was marked as spam.

Copy link
Copy Markdown
Collaborator

@basvandijk basvandijk left a comment

Choose a reason for hiding this comment

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

I remember there was some reason to mark them as testonly but can't remember the details. But I guess if CI passes it's good!


rust_library(
name = "test_utils",
testonly = True,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A lot of them, like this one, is in a directory containing test_utils. Shouldn't we keep them for those?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I'll need to double check this. Might have been too zealous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants