Skip to content

Conversation

@TysonRayJones
Copy link
Contributor

@TysonRayJones TysonRayJones commented Mar 15, 2025

Hello beautiful Catch2 team!

This PR patches a minor formatting bug in the base reporter due to this line in catch_reporter_helpers.cpp, which includes:

std::setw( 2 ) << tagCount.count

Passing the CLI argument --list-tags outputs a list like:

All available tags:
  11  [mytag]
   4  [myothertag]
2 tags

However, the width of the tag-count column was previously hardcoded to 2 (as per the line above), such that the formatting broke (with the tag names awkwardly misaligned/displaced) for 3+ digit tag counts. This occurred whenever a tag contains one hundred or more tests (like in my use-case for scientific software tests), and would result in formatting like:

All available tags:
  11  [mytag]
  100  [myjumbotag]
   4  [myothertag]
  4312  [myunbelievablypopulartag]
3 tags

This patch pre-computes the maximum number of digits among the tag counts, expanding the padding when there are more than 100 tests in a tag (and handling when tags are empty). It now outputs e.g.

All available tags:
    11  [mytag]
   100  [myjumbotag]
     4  [myothertag]
  4312  [myunbelievablypopulartag]
3 tags

I have endeavoured to replicate the format of clang-format, modelled from a code snippet just above the diff performing a similar chore (defaultListListeners choosing the padding of the listener name column). Note I used an explicit size_t type over auto for the "main" variable maxTagCountLen to inform the static_cast, necessarily because std::floor returns a non-integral type (e.g. double) which cannot be passed to std::setw.

This abominable MWE can be used to generate many same-tag tests to check the formatting:

#include <catch2/catch_test_macros.hpp>

// Mn(tag) instantiates 3^(n-1) uniquely-named tests with the given tag
#define M1( name, tag ) TEST_CASE( name, tag ) { }
#define M2( name, tag ) M1( name, tag ) M1( name "a", tag ) M1( name "aa", tag )
#define M3( name, tag ) M2( name, tag ) M2( name "b", tag ) M2( name "bb", tag )
#define M4( name, tag ) M3( name, tag ) M3( name "c", tag ) M3( name "cc", tag )
#define M5( name, tag ) M4( name, tag ) M4( name "d", tag ) M4( name "dd", tag )
#define M6( name, tag ) M5( name, tag ) M5( name "e", tag ) M5( name "ee", tag )
#define M7( name, tag ) M6( name, tag ) M6( name "f", tag ) M6( name "ff", tag )
#define M8( name, tag ) M7( name, tag ) M7( name "g", tag ) M7( name "gg", tag )

// invoke as many macros as you dare!
M1("1", "[tag1]");
M2("2", "[tag2]");
M3("3", "[tag3]");
M4("4", "[tag4]");
M5("5", "[tag5]");
M6("6", "[tag6]"); // bug visible from M6
M7("7", "[tag7]");
M8("8", "[tag8]");

// then run ./exec --list-tags

Previously this MWE would erroneously output:

All available tags:
  1   [tag1]
  3   [tag2]
  9   [tag3]
  27  [tag4]
  81  [tag5]
  243  [tag6]
  729  [tag7]
  2187  [tag8]
8 tags

but under this patch, now outputs:

All available tags:
     1  [tag1]
     3  [tag2]
     9  [tag3]
    27  [tag4]
    81  [tag5]
   243  [tag6]
   729  [tag7]
  2187  [tag8]
8 tags

Finally, invoking no macros correctly outputs:

All available tags:
0 tags

All things considered, this solves an extremely minor issue, but I'm thrilled in any case to be able to give back to a project I adore!
Tyson

Passing the CLI argument '--list-tags' outputs a list like:

```
All available tags:
  11  [mytag]
   4  [myothertag]
2 tags
```

However, the width of the tag-count column was previously hardcoded to 2, such that the formatting broke (with the tag names awkwardly misaligned/displaced) for 3+ digit tag counts. This occurred whenever a tag contains one hundred or more tests, and would result in formatting like:

```
All available tags:
  11  [mytag]
  100  [myjumbotag]
   4  [myothertag]
3 tags
```

This patch pre-computes the maximum number of digits among the tag counts, expanding the padding when there are more than 100 tests in a tag (and handling when tags are empty). It now outputs e.g.

```
All available tags:
   11  [mytag]
  100  [myjumbotag]
    4  [myothertag]
3 tags
```
@TysonRayJones TysonRayJones changed the title patched >2-digit tag-count padding in base reporter Patch >2-digit tag-count padding in base reporter Mar 15, 2025
@codecov
Copy link

codecov bot commented Mar 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.99%. Comparing base (76f70b1) to head (0c72039).
Report is 36 commits behind head on devel.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #2963      +/-   ##
==========================================
+ Coverage   90.98%   90.99%   +0.01%     
==========================================
  Files         198      198              
  Lines        8599     8609      +10     
==========================================
+ Hits         7823     7833      +10     
  Misses        776      776              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@shahsb shahsb left a comment

Choose a reason for hiding this comment

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

LGTM.

@horenmar
Copy link
Member

horenmar commented Jul 2, 2025

Thanks, lot of effort obviously went to the PR description and commit message.

Normally I would ask for a test, but since this has been opened few months ago, and the code change is obviously fairly harmless, I'll merge it as is.

@horenmar horenmar merged commit bd2d918 into catchorg:devel Jul 2, 2025
3 checks passed
@horenmar horenmar added the BugFix label Jul 2, 2025
GerHobbelt added a commit to GerHobbelt/Catch2 that referenced this pull request Aug 27, 2025
v3.9.0

=== Improvements ===
* **Added experimental opt-in support for thread safe assertions**
  * Read the documentation for full details
* **The default test run order has been changed to random**
* Passing assertions are significantly faster when the reporter does not ask for `assertionEnded` events on passing assertions.
  * This is the default behaviour of e.g. Console or Compact reporter
  * Simple `REQUIRE(true)` is 60% faster in Release and 80% faster in Debug build configuration
  * Simple `REQUIRE_NOTHROW` is 230% faster in Release and 430% faster in Debug build configuration
  * Simple `REQUIRE_THROWS` is ~3% faster in Release and 20% faster in Debug build configuration (throwing introduces enough overhead that the optimizations inside Catch2 are mostly irrelevant)
* Small (2-5%) improvement if the reporter asks for `assertionEnded` events for passing assertions.
* The exit code constants are part of the Session API. (catchorg#2955, catchorg#2976)
* Suppressed unsigned integer overflow checking in locations with intended overflow (catchorg#2965)
* Reporters flush output after writing metadata, e.g. rng seed (catchorg#2964)
* Added unreachable after `FAIL` and `SKIP` macros (catchorg#2941)
  * This allows the compiler to understand that the execution does not continue past the macro, and avoids warnings.
* Added fast path for `assertionStarting` event when no reporter requires it
  * For backwards compatibility, this fast path is opt-in
  * A reporter can opt in by changing its `ReporterPreferences::shouldReportAllAssertionStarts`
* Improved last seen source location tracking to be more precise
  * This is used when reporting unexpected exceptions from tests

=== Fixes ===
* Fixed formatting of tags with more than 100 tests in the default `--list-tags` output (catchorg#2963)
* Fixed Clang-Tidy's `readability-static-accessed-through-instance` in tests
* Fixed most of Clang-Tidy's `cppcoreguidelines-avoid-non-const-global-variables` (catchorg#2582)
* The lifetime of scoped messages now strictly obeys their scope (catchorg#1759, catchorg#2019, catchorg#2959)
  * Previously Catch2 would try to keep them around during unexpected exception, to provide helpful context.
  * The amount of surprises the irregularities caused was not worth the occasional utility provided.
* `TEMPLATE_TEST_CASE_SIG` can handle signatures consisting of only types (catchorg#2680, catchorg#2995)
* Moved `catch_test_run_info.hpp` up from `internal/` subfolder into the main one (catchorg#2972)

=== Miscellaneous ===
* pkg-config files are now generated at install time (catchorg#2979)
  * This fixes missing debug suffix in library names
  * This fixes install prefix mismatch between build config and actuall installation

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCgAdFiEE8QyLZSqAHw/oZQgX3kgwe4sNOBoFAmiCkWIACgkQ3kgwe4sN
# OBrohA//VM0MoWEyb1Mp9Lw4pw07CL7EnqsgOnOjEu1EgN1tyDMEw3vf/uysiY4X
# ab7RwVzCd0WVnixmNsduhzotCC0o4cDupPNXMTiRgO7ijtzLOfCsUqc+ZuIP0xUJ
# a1IcokrQM/FYPfltBfJUCQdpWHX/Co0dDNtXlCjY2Ur3Ek/4w4M8efARKZi1xi+C
# WDprUrOcYCWlYSzRa7LXWbkoCETdEphQCIXfCo8/kIbIf8+roJuiH/9RE6UrdeDY
# yQfH6qLUoPdfeIQGGJY+dN+rVfsQkBBM2V6dsYcNsAXWsHEzDFJhI1KQHNw51T+3
# MSaGmUKa3ptwW8p+ecEsqdSiygMBmrVW+x1Oqk/FCuW1r7lDemnh9ZjyIt2PRfxO
# 6AxTepyT2wJhjHK/UecIxhh7Xba7Cbbu9vT3FplJFtLI4MVgXFpo4hFKHGl2Y+oI
# okJgoDb+Bh2sWqqFQdKfIq+0ntn8Oqfas64QV7jmT/M1f9ENMqrunEnmQzPGcu8t
# ckly4+WVTCXAoSseKWJasJgFCPQjrjQYcaEtQNCNyYLSjLFJIMNmXp5NrlSmtLHv
# va7JIMLEgar4kMuiRUeu3Tz0mDrWti4/eX503vBs/BF8/O8m6XKnv7vXcVhg2mQW
# pLT5qM8bbAv0AnxHFgewD2ztuEoTPECqq6T06t77EHYDTPhys/4=
# =6rlc
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu Jul 24 22:02:42 2025 WEDT
# gpg:                using RSA key F10C8B652A801F0FE8650817DE48307B8B0D381A
# gpg: Can't check signature: No public key

# Conflicts:
#	extras/catch_amalgamated.cpp
#	extras/catch_amalgamated.hpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants