testpackage icon indicating copy to clipboard operation
testpackage copied to clipboard

Only flag when a test package only tests public methods

Open G-Rath opened this issue 9 months ago • 2 comments

While I really love the idea, sometimes we do "have" to test internals and with large codebases it can be tedious to manage the allow-packages.

However I think in practice most people just default to not having their tests in a public package rather than needing to actually test something private, so it would be useful if we could have this linter report when a test package only uses public elements and thus could be made into its own package.

I'm not sure how feasible that actually is though since it would require the linter to consider the whole package rather than individual files

G-Rath avatar Mar 26 '25 22:03 G-Rath

The original intention was to force people to use private stuff in tests as little as possible. With a proposed option, the linter becomes useless, doesn't it?

maratori avatar Apr 13 '25 20:04 maratori

I don't think so because changing between the two requires work as packages are not allowed to self-reference; for example with the following:

package database_test

import (
	"testing"

	"github.com/g-rath/osv-detector/pkg/database"
)

func TestNewAPIDB_Valid(t *testing.T) {
	t.Parallel()

	config := database.Config{URL: "https://my-api.com", Name: "my-api"}

	db, err := database.NewAPIDB(config, false, 100)

	if err != nil {
		t.Errorf("NewAPIDB() unexpected error \"%v\"", err)
	}

	if db == nil {
		t.Fatalf("NewAPIDB() db unexpectedly nil")
	}

	// ...
}

If I was to change that to be package database, then Go will force me to remove the self-import and then change all my database.* references to drop the database, which could easily be a lot of work making both authors and reviewers think about if this is worth that effort and hopefully think maybe they should do something different.

I agree that this does weaken the linter, and it would in particular be a bit useless when you're creating a brand new package, but imo it would still be better than not having the linter at all

G-Rath avatar Apr 13 '25 21:04 G-Rath