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

Patch >2-digit tag-count padding in base reporter #2963

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

TysonRayJones
Copy link

@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
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).

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants