jbloomAus / SAELens

Training Sparse Autoencoders on Language Models
https://jbloomaus.github.io/SAELens/
MIT License
481 stars 127 forks source link

[Proposal] Move files in `tests/unit` to `tests` #335

Open anthonyduong9 opened 1 month ago

anthonyduong9 commented 1 month ago

Proposal

Move files in tests/unit to tests, delete tests/acceptance, and move files in tests/benchmark to benchmark.

Motivation

Most of the tests in test/unit are technically integration tests since we don't mock out dependencies. We just want to make sure we have test coverage, whether it's from unit tests or integration tests, and it doesn't matter if we don't split unit tests and integration tests since the codebase isn't large. We should just call them "tests", but tests in tests/unit are run in CI, and tests in tests/benchmark aren't. We want people to add tests that are run by CI, and not add tests which aren't.

The directory name unit is not technically correct and is confusing.

Pitch

We move files in tests/unit to tests, delete tests/acceptance, and move files in tests/benchmark to benchmark, and run the tests in tests in CI.

Alternatives

We split the tests in tests/unit into unit tests and integration tests.

Additional context

The proposal is from https://github.com/jbloomAus/SAELens/pull/331#discussion_r1799116941.

Checklist

chanind commented 1 month ago

👍 I support this!