Skip to content

tests: drop mk-bundle exceptions #17468

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

Closed
wants to merge 157 commits into from
Closed

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented May 27, 2025

Using a mixture of techniques to avoid symbols collisions:

  • reduce scope.
  • add t*_ / T*_ prefix.
  • move shared functions to testutil.c.
    (suburl(), rlim2str())
  • clone re-used lib*.c sources.
    (lib587, lib645)
  • include shared symbols just once in re-used lib*.c sources.
    (using LIB*_C guards.)
  • drop re-used lib*.c sources where they were identical or
    unused.
  • make macros global.
  • #undef macros before use.

What remain is the entry functions test, and unit_setup,
unit_stop in unit tests.

Also:

  • fix formatting and other minor things along the way.
  • add const where possible.
  • sync some symbol names between tests.
  • drop mk-bundle-hints.sh that's no longer necessary.

TODO:

  • enable test bundles by default.

@vszakats vszakats added the tests label May 27, 2025
@vszakats vszakats marked this pull request as draft May 27, 2025 21:25
Copy link
Contributor

@dfandrich dfandrich left a comment

Choose a reason for hiding this comment

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

On one hand, it's good that the macro undefs are now explicit and not magic, but on the other hand it's not obvious by looking why they're needed. On balance, less magic is probably more clear.

@vszakats
Copy link
Member Author

vszakats commented May 27, 2025

Possible tweaking options:

  • prefix SAFETY_MARGIN with Tn_ (2 uses), to avoid the #undefs.
  • move NUM_HANDLES to test.h, maybe, to avoid most #undefs. It's used in 8 .c source. (Some of them re-used for multiple tests, which makes prefixing less practical in this case).

Then come the other symbols. Where the tricky case is re-used sources:

lib500.c   2x
lib510.c   2x
lib525.c   2x
lib526.c   3x
lib544.c   2x
lib547.c   2x
lib554.c   2x
lib556.c   2x
lib589.c   2x
lib643.c   2x
lib670.c   4x
lib1502.c  4x
lib1514.c  2x
lib1518.c  2x
lib1576.c  2x
lib1594.c  2x
lib1916.c  2x
lib1940.c  2x

@testclutch

This comment was marked as outdated.

@vszakats vszakats force-pushed the t-bundle-lessexcp branch from b4a494c to bae721a Compare June 2, 2025 22:32
@vszakats vszakats changed the title tests: reduce mk-bundle exceptions tests: drop mk-bundle exceptions Jun 3, 2025
@vszakats vszakats marked this pull request as ready for review June 3, 2025 09:52
@vszakats vszakats force-pushed the t-bundle-lessexcp branch from 65497ac to d13432e Compare June 5, 2025 07:47
@vszakats vszakats closed this in 6897aeb Jun 11, 2025
@vszakats vszakats deleted the t-bundle-lessexcp branch June 11, 2025 03:40
vszakats added a commit that referenced this pull request Jun 11, 2025
After this patch there is no more double/multiple compile of the same
libtest source under a different libtest ID. Each libtest is compiled
once, and changing behavior at runtime based on test ID.

- drop recently added physical clones for two prevously multi-compiled
  tests:
  - merge lib587 into lib554 again, branch at runtime.
  - merge lib645 into lib643 again, branch at runtime.

- replace existing dynamic branching to use `testnum` instead of
  a manually rolled `testno` based on an extra command-line argument.
  lib1571, lib1576.

- mk-bundle.pl: stop defining `LIB*` macros. No longer used.

- libtests: drop all `LIB*_C` guards.

- Make these tests branch at runtime, stop building copies:
  - lib585, based on lib500
  - lib565, based on lib510
  - lib529, based on lib525
  - lib527, lib532, based on lib526
  - lib545, based on lib544
  - lib548, based on lib547
  - lib696, based on lib556
  - lib584, based on lib589
  - lib1539, based on lib1514
  - lib1543, based on lib1518
  - lib1917, based on lib1916
  - lib1946, based on lib1940
  - lib671, 672, 673, based on lib670

Follow-up to 02dd471 #17591
Follow-up to 6897aeb #17468

Closes #17598
vszakats added a commit that referenced this pull request Jun 14, 2025
Make test bundles the default. Drop non-bundle build mode.
Also do all the optimizations and tidy-ups this allows, simpler builds,
less bundle exceptions, streamlined build mechanics.

Also rework the init/deinit macro magic for unit tests. The new method
allows using unique init/deinit function names, and calling them with
arguments. This is in turn makes it possible to reduce the use of global
variables.

Note this drop existing build options `-DCURL_TEST_BUNDLES=` from cmake
and `--enable-test-bundles` / `--disable-test-bundles` from autotools.

Also:
- rename test entry functions to have unique names: `test_<testname>`
  This removes the last exception that was handled in the generator.
- fix `make dist` to not miss test sources with test bundles enabled.
- sync and merge `tests/mk-bundle.pl` into `scripts/mk-unity.pl`.
- mk-unity.pl: add `--embed` option and use it when `CURL_CLANG_TIDY=ON`
  to ensure that `clang-tidy` does not miss external test C sources.
  (because `clang-tidy` ignores code that's #included.)
- tests/unit: drop no-op setup/stop functions.
- tests: reduce symbol scopes, global macros, other fixes and tidy-ups.
- tool1621: fix to run, also fix it to pass.
- sockfilt: fix Windows compiler warning in certain unity include order,
  by explicitly including `warnless.h`.

Follow-up to 6897aeb #17468

Closes #17590
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
Using a mixture of techniques to avoid symbols collisions:
- reduce scope.
- add `t*_` / `T*_` prefix.
- move shared functions to `testutil.c`.
  (`suburl()`, `rlim2str()`)
- clone re-used lib*.c sources.
  (lib587, lib645)
- include shared symbols just once in re-used `lib*.c` sources.
  (using `LIB*_C` guards.)
- drop re-used `lib*.c` sources where they were identical or
  unused.
- make macros global.
- #undef macros before use.

What remain is the entry functions `test`, and `unit_setup`,
`unit_stop` in unit tests.

Also:
- fix formatting and other minor things along the way.
- add `const` where possible.
- sync some symbol names between tests.
- drop `mk-bundle-hints.sh` that's no longer necessary.

Closes curl#17468
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
After this patch there is no more double/multiple compile of the same
libtest source under a different libtest ID. Each libtest is compiled
once, and changing behavior at runtime based on test ID.

- drop recently added physical clones for two prevously multi-compiled
  tests:
  - merge lib587 into lib554 again, branch at runtime.
  - merge lib645 into lib643 again, branch at runtime.

- replace existing dynamic branching to use `testnum` instead of
  a manually rolled `testno` based on an extra command-line argument.
  lib1571, lib1576.

- mk-bundle.pl: stop defining `LIB*` macros. No longer used.

- libtests: drop all `LIB*_C` guards.

- Make these tests branch at runtime, stop building copies:
  - lib585, based on lib500
  - lib565, based on lib510
  - lib529, based on lib525
  - lib527, lib532, based on lib526
  - lib545, based on lib544
  - lib548, based on lib547
  - lib696, based on lib556
  - lib584, based on lib589
  - lib1539, based on lib1514
  - lib1543, based on lib1518
  - lib1917, based on lib1916
  - lib1946, based on lib1940
  - lib671, 672, 673, based on lib670

Follow-up to 02dd471 curl#17591
Follow-up to 6897aeb curl#17468

Closes curl#17598
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
Make test bundles the default. Drop non-bundle build mode.
Also do all the optimizations and tidy-ups this allows, simpler builds,
less bundle exceptions, streamlined build mechanics.

Also rework the init/deinit macro magic for unit tests. The new method
allows using unique init/deinit function names, and calling them with
arguments. This is in turn makes it possible to reduce the use of global
variables.

Note this drop existing build options `-DCURL_TEST_BUNDLES=` from cmake
and `--enable-test-bundles` / `--disable-test-bundles` from autotools.

Also:
- rename test entry functions to have unique names: `test_<testname>`
  This removes the last exception that was handled in the generator.
- fix `make dist` to not miss test sources with test bundles enabled.
- sync and merge `tests/mk-bundle.pl` into `scripts/mk-unity.pl`.
- mk-unity.pl: add `--embed` option and use it when `CURL_CLANG_TIDY=ON`
  to ensure that `clang-tidy` does not miss external test C sources.
  (because `clang-tidy` ignores code that's #included.)
- tests/unit: drop no-op setup/stop functions.
- tests: reduce symbol scopes, global macros, other fixes and tidy-ups.
- tool1621: fix to run, also fix it to pass.
- sockfilt: fix Windows compiler warning in certain unity include order,
  by explicitly including `warnless.h`.

Follow-up to 6897aeb curl#17468

Closes curl#17590
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants