Skip to content
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

Split tests into public and internal ones #1037

Open
sipa opened this issue Dec 7, 2021 · 1 comment
Open

Split tests into public and internal ones #1037

sipa opened this issue Dec 7, 2021 · 1 comment

Comments

@sipa
Copy link
Contributor

sipa commented Dec 7, 2021

The current unit tests are all built against the library's internal source code directly. That means they have the advantage of being to test internal functions that aren't exposed from the library, but it also means the tests cannot actually run against the normal release build of the library.

To improve this:

  • Split the tests into separate public API tests, and internal tests.
  • A public test binary: public API tests, linked against the release build of the library (no -DVERIFY).
  • An internal test binary: #include's the library code directly, plus public API tests, plus internal test (compiled with -DVERIFY).
@real-or-random
Copy link
Contributor

real-or-random commented Jan 5, 2023

I also see some value in running the internal tests without -DVERIFY. This would exercise a lot of functions with their production source.

That doesn't mean that we'll get the same compiler output as in production (but probably the compiler will be closer to production at least). And it will catch cases in our source code, in which VERIFY code introduces real differences (which it should not).

So it's not perfect but tests are never perfect. I think this is a low hanging fruit, given that it should not be a lot of effort. It will just be a second internal test binary.

sipa added a commit that referenced this issue Jan 9, 2023
…t VERIFY

2037600 tests: Add noverify_tests which is like tests but without VERIFY (Tim Ruffing)

Pull request description:

  mentioned in #1037 (comment)

  Let's see how this affects CI time

ACKs for top commit:
  sipa:
    ACK 2037600
  apoelstra:
    ACK 2037600

Tree-SHA512: fab1ce1499d418671d3d0ecfddf15d75b7c2bbfbfb4be958a95730491244185a906c7133aba4d0bec56ee6c721cb525750eef4cafc12f386484af931e34b0e8e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants