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

Report generator states for failed assertions #2509

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/311-Gen-CustomCapture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ TEST_CASE("Generate random doubles across different ranges",
// This will take r1 by reference and r2 by value.
// Note that there are no advantages for doing so in this example,
// it is done only for expository purposes.
auto number = Catch::Generators::generate( "custom capture generator", CATCH_INTERNAL_LINEINFO,
auto number = Catch::Generators::generate( "custom capture generator", "custom arguments description", CATCH_INTERNAL_LINEINFO,
[&r1, r2]{
using namespace Catch::Generators;
return makeGenerators(take(50, random(std::get<0>(r1), std::get<1>(r2))));
Expand Down
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ set(INTERNAL_HEADERS
${SOURCES_DIR}/matchers/catch_matchers_templated.hpp
${SOURCES_DIR}/matchers/catch_matchers_vector.hpp
${SOURCES_DIR}/catch_message.hpp
${SOURCES_DIR}/internal/catch_generator_info.hpp
${SOURCES_DIR}/internal/catch_message_info.hpp
${SOURCES_DIR}/internal/catch_meta.hpp
${SOURCES_DIR}/internal/catch_move_and_forward.hpp
Expand Down Expand Up @@ -236,6 +237,7 @@ set(IMPL_SOURCES
${SOURCES_DIR}/internal/catch_errno_guard.cpp
${SOURCES_DIR}/internal/catch_lazy_expr.cpp
${SOURCES_DIR}/internal/catch_leak_detector.cpp
${SOURCES_DIR}/internal/catch_generator_info.cpp
${SOURCES_DIR}/internal/catch_message_info.cpp
${SOURCES_DIR}/internal/catch_polyfills.cpp
${SOURCES_DIR}/internal/catch_startup_exception_registry.cpp
Expand Down
1 change: 1 addition & 0 deletions src/catch2/catch_all.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
#include <catch2/internal/catch_exception_translator_registry.hpp>
#include <catch2/internal/catch_fatal_condition_handler.hpp>
#include <catch2/internal/catch_floating_point_helpers.hpp>
#include <catch2/internal/catch_generator_info.hpp>
#include <catch2/internal/catch_istream.hpp>
#include <catch2/internal/catch_lazy_expr.hpp>
#include <catch2/internal/catch_leak_detector.hpp>
Expand Down
7 changes: 6 additions & 1 deletion src/catch2/generators/catch_generators.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <catch2/catch_tostring.hpp>
#include <catch2/interfaces/catch_interfaces_generatortracker.hpp>
#include <catch2/internal/catch_generator_info.hpp>
#include <catch2/internal/catch_source_line_info.hpp>
#include <catch2/internal/catch_stringref.hpp>
#include <catch2/internal/catch_move_and_forward.hpp>
Expand Down Expand Up @@ -210,7 +211,7 @@ namespace Detail {
// Note: The type after -> is weird, because VS2015 cannot parse
// the expression used in the typedef inside, when it is in
// return type. Yeah.
auto generate( StringRef generatorName, SourceLineInfo const& lineInfo, L const& generatorExpression ) -> decltype(std::declval<decltype(generatorExpression())>().get()) {
auto generate( StringRef generatorName, StringRef argumentsDescription, SourceLineInfo const& lineInfo, L const& generatorExpression ) -> decltype(std::declval<decltype(generatorExpression())>().get()) {
using UnderlyingType = typename decltype(generatorExpression())::type;

IGeneratorTracker& tracker = acquireGeneratorTracker( generatorName, lineInfo );
Expand All @@ -219,6 +220,7 @@ namespace Detail {
}

auto const& generator = static_cast<IGenerator<UnderlyingType> const&>( *tracker.getGenerator() );
getResultCapture().trackGeneratorState(GeneratorInfo(generatorName, argumentsDescription, lineInfo, generator.currentElementAsString()));
Copy link
Author

Choose a reason for hiding this comment

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

I was wondering if there's maybe a "cheaper" or better way to only get the generator states, if the test fails.

return generator.get();
}

Expand All @@ -227,14 +229,17 @@ namespace Detail {

#define GENERATE( ... ) \
Catch::Generators::generate( INTERNAL_CATCH_STRINGIZE(INTERNAL_CATCH_UNIQUE_NAME(generator)), \
INTERNAL_CATCH_STRINGIZE( __VA_ARGS__ ), \
CATCH_INTERNAL_LINEINFO, \
[ ]{ using namespace Catch::Generators; return makeGenerators( __VA_ARGS__ ); } ) //NOLINT(google-build-using-namespace)
#define GENERATE_COPY( ... ) \
Catch::Generators::generate( INTERNAL_CATCH_STRINGIZE(INTERNAL_CATCH_UNIQUE_NAME(generator)), \
INTERNAL_CATCH_STRINGIZE( __VA_ARGS__ ), \
CATCH_INTERNAL_LINEINFO, \
[=]{ using namespace Catch::Generators; return makeGenerators( __VA_ARGS__ ); } ) //NOLINT(google-build-using-namespace)
#define GENERATE_REF( ... ) \
Catch::Generators::generate( INTERNAL_CATCH_STRINGIZE(INTERNAL_CATCH_UNIQUE_NAME(generator)), \
INTERNAL_CATCH_STRINGIZE( __VA_ARGS__ ), \
CATCH_INTERNAL_LINEINFO, \
[&]{ using namespace Catch::Generators; return makeGenerators( __VA_ARGS__ ); } ) //NOLINT(google-build-using-namespace)

Expand Down
2 changes: 2 additions & 0 deletions src/catch2/interfaces/catch_interfaces_capture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace Catch {
struct AssertionInfo;
struct SectionInfo;
struct SectionEndInfo;
struct GeneratorInfo;
struct MessageInfo;
struct MessageBuilder;
struct Counts;
Expand All @@ -43,6 +44,7 @@ namespace Catch {
virtual void sectionEndedEarly( SectionEndInfo const& endInfo ) = 0;

virtual auto acquireGeneratorTracker( StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker& = 0;
virtual void trackGeneratorState( GeneratorInfo const& info ) = 0;

virtual void benchmarkPreparing( StringRef name ) = 0;
virtual void benchmarkStarting( BenchmarkInfo const& info ) = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/catch2/interfaces/catch_interfaces_reporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ namespace Catch {

AssertionStats::AssertionStats( AssertionResult const& _assertionResult,
std::vector<MessageInfo> const& _infoMessages,
std::vector<GeneratorInfo> const& _generatorInfos,
Totals const& _totals )
: assertionResult( _assertionResult ),
infoMessages( _infoMessages ),
generatorInfos( _generatorInfos ),
totals( _totals )
{
assertionResult.m_resultData.lazyExpression.m_transientExpression = _assertionResult.m_resultData.lazyExpression.m_transientExpression;
Expand Down
3 changes: 3 additions & 0 deletions src/catch2/interfaces/catch_interfaces_reporter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <catch2/catch_totals.hpp>
#include <catch2/catch_assertion_result.hpp>
#include <catch2/internal/catch_message_info.hpp>
#include <catch2/internal/catch_generator_info.hpp>
#include <catch2/internal/catch_stringref.hpp>
#include <catch2/internal/catch_unique_ptr.hpp>
#include <catch2/internal/catch_move_and_forward.hpp>
Expand Down Expand Up @@ -65,6 +66,7 @@ namespace Catch {
struct AssertionStats {
AssertionStats( AssertionResult const& _assertionResult,
std::vector<MessageInfo> const& _infoMessages,
std::vector<GeneratorInfo> const& _generatorInfos,
Totals const& _totals );

AssertionStats( AssertionStats const& ) = default;
Expand All @@ -74,6 +76,7 @@ namespace Catch {

AssertionResult assertionResult;
std::vector<MessageInfo> infoMessages;
std::vector<GeneratorInfo> generatorInfos;
Totals totals;
};

Expand Down
1 change: 1 addition & 0 deletions src/catch2/internal/catch_console_colour.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ namespace Catch {

OriginalExpression = Cyan,
ReconstructedExpression = BrightYellow,
GeneratorValue = Cyan,

SecondaryText = LightGrey,
Headers = White
Expand Down
22 changes: 22 additions & 0 deletions src/catch2/internal/catch_generator_info.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

// Copyright Catch2 Authors
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// https://www.boost.org/LICENSE_1_0.txt)

// SPDX-License-Identifier: BSL-1.0

#include <catch2/internal/catch_generator_info.hpp>

namespace Catch {

GeneratorInfo::GeneratorInfo( StringRef _name,
StringRef _arguments,
SourceLineInfo const& _lineInfo,
StringRef _currentElement ):
name( _name ),
arguments( _arguments ),
lineInfo( _lineInfo ),
currentElement( _currentElement ) {}

} // end namespace Catch
37 changes: 37 additions & 0 deletions src/catch2/internal/catch_generator_info.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@

// Copyright Catch2 Authors
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// https://www.boost.org/LICENSE_1_0.txt)

// SPDX-License-Identifier: BSL-1.0
#ifndef CATCH_GENERATOR_INFO_HPP_INCLUDED
#define CATCH_GENERATOR_INFO_HPP_INCLUDED

#include <catch2/interfaces/catch_interfaces_capture.hpp>
#include <catch2/internal/catch_source_line_info.hpp>
#include <string>

namespace Catch {

struct GeneratorInfo {
GeneratorInfo( StringRef _name,
StringRef _arguments,
SourceLineInfo const& _lineInfo,
StringRef currentElement );

StringRef name;
StringRef arguments;
SourceLineInfo lineInfo;
StringRef currentElement;

bool operator==( GeneratorInfo const& other ) const {
return name == other.name && arguments == other.arguments &&
lineInfo == other.lineInfo &&
currentElement == other.currentElement;
}
};

} // end namespace Catch

#endif // CATCH_GENERATOR_INFO_HPP_INCLUDED
14 changes: 13 additions & 1 deletion src/catch2/internal/catch_run_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ namespace Catch {
m_lastAssertionPassed = true;
}

m_reporter->assertionEnded(AssertionStats(result, m_messages, m_totals));
m_reporter->assertionEnded(AssertionStats(result, m_messages, m_generatorInfos, m_totals));

if (result.getResultType() != ResultWas::Warning)
m_messageScopes.clear();
Expand Down Expand Up @@ -319,6 +319,16 @@ namespace Catch {
return tracker;
}

void RunContext::trackGeneratorState( GeneratorInfo const& info ) {
// Avoid redundant entries, in case a generator is used within a loop.
Copy link
Author

Choose a reason for hiding this comment

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

I added the unique name to the info, so this comparison should be fine even if we have more than one generator with the same arguments on the same line. Both of them should show up. This should be tested. Where would the test go? I assume some test that runs through via the approval tests.

if ( std::find( m_generatorInfos.cbegin(),
Copy link
Author

Choose a reason for hiding this comment

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

Since info.name is already unique, we could use find_if based on the name only. Saves extra some comparisons.

m_generatorInfos.cend(),
info ) != m_generatorInfos.cend() )
return;

m_generatorInfos.push_back( info );
}

bool RunContext::testForMissingAssertions(Counts& assertions) {
if (assertions.total() != 0)
return false;
Expand All @@ -343,6 +353,7 @@ namespace Catch {
m_reporter->sectionEnded(SectionStats(endInfo.sectionInfo, assertions, endInfo.durationInSeconds, missingAssertions));
m_messages.clear();
m_messageScopes.clear();
m_generatorInfos.clear();
}

void RunContext::sectionEndedEarly(SectionEndInfo const & endInfo) {
Expand Down Expand Up @@ -490,6 +501,7 @@ namespace Catch {
handleUnfinishedSections();
m_messages.clear();
m_messageScopes.clear();
m_generatorInfos.clear();

SectionStats testCaseSectionStats(testCaseSection, assertions, duration, missingAssertions);
m_reporter->sectionEnded(testCaseSectionStats);
Expand Down
2 changes: 2 additions & 0 deletions src/catch2/internal/catch_run_context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ namespace Catch {
void sectionEndedEarly( SectionEndInfo const& endInfo ) override;

auto acquireGeneratorTracker( StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker& override;
void trackGeneratorState( GeneratorInfo const& info ) override;

void benchmarkPreparing( StringRef name ) override;
void benchmarkStarting( BenchmarkInfo const& info ) override;
Expand Down Expand Up @@ -132,6 +133,7 @@ namespace Catch {
Totals m_totals;
IEventListenerPtr m_reporter;
std::vector<MessageInfo> m_messages;
std::vector<GeneratorInfo> m_generatorInfos;
std::vector<ScopedMessage> m_messageScopes; /* Keeps owners of so-called unscoped messages. */
AssertionInfo m_lastAssertionInfo;
std::vector<SectionEndInfo> m_unfinishedSections;
Expand Down
16 changes: 16 additions & 0 deletions src/catch2/reporters/catch_reporter_console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class ConsoleAssertionPrinter {
printResultType();
printOriginalExpression();
printReconstructedExpression();
printGeneratorStates();
} else {
stream << '\n';
}
Expand Down Expand Up @@ -154,6 +155,21 @@ class ConsoleAssertionPrinter {
<< '\n';
}
}
void printGeneratorStates() const {
if ( stats.generatorInfos.empty() ) {
return;
}

stream << "with " << pluralise(stats.generatorInfos.size(), "generator"_sr) << '\n';
for ( auto const& info : stats.generatorInfos ) {
stream << TextFlow::Column( "line:" ).indent( 2 )
<< info.lineInfo.line << ": "
<< "GENERATE(" << info.arguments << ")\n"
<< TextFlow::Column( "value: " ).indent( 2 )
<< colourImpl->guardColour( Colour::GeneratorValue )
<< info.currentElement << '\n';
}
}
void printMessage() const {
if (!messageLabel.empty())
stream << messageLabel << ':' << '\n';
Expand Down