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.

Sign up for GitHub

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

Jump to bottom

Make SyntaxTest easier to extend with custom expectations #14505

Merged
merged 5 commits into from
Aug 23, 2023
Merged

Make SyntaxTest easier to extend with custom expectations #14505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c5b81b6
Define Error::severity()
cameel Aug 21, 2023
b1ead4a
Order Error::Severity enum from most to least severe
cameel Aug 21, 2023
73b9077
SyntaxTest: Default-initialize boolean fields without showing the value
cameel Aug 21, 2023
c965d63
SyntaxTest: Allow derived test cases to filter out warnings and infos
cameel Aug 21, 2023
e847596
CommonSyntaxTest: Add support for syntax tests with custom expectatio…
cameel Aug 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions liblangutil/Exceptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ class Error: virtual public util::Exception
public:
enum class Type
{
Info,
Warning,
CodeGenerationError,
DeclarationError,
DocstringParsingError,
Expand All @@ -185,15 +187,14 @@ class Error: virtual public util::Exception
UnimplementedFeatureError,
YulException,
SMTLogicException,
Warning,
Info
};

enum class Severity
{
Error,
// NOTE: We rely on these being ordered from least to most severe.
Info,
Warning,
Info
Error,
};

Error(
Expand All @@ -206,6 +207,7 @@ class Error: virtual public util::Exception

ErrorId errorId() const { return m_errorId; }
Type type() const { return m_type; }
Severity severity() const { return errorSeverity(m_type); }

SourceLocation const* sourceLocation() const noexcept;
SecondarySourceLocation const* secondarySourceLocation() const noexcept;
Expand Down
46 changes: 42 additions & 4 deletions test/CommonSyntaxTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <test/CommonSyntaxTest.h>
#include <test/Common.h>
#include <test/TestCase.h>
#include <libsolutil/CommonIO.h>
#include <boost/algorithm/string.hpp>
#include <boost/algorithm/string/predicate.hpp>
#include <boost/test/unit_test.hpp>
Expand All @@ -29,6 +30,7 @@

using namespace std;
using namespace solidity;
using namespace solidity::util;
using namespace solidity::util::formatting;
using namespace solidity::langutil;
using namespace solidity::frontend;
Expand Down Expand Up @@ -66,14 +68,15 @@ CommonSyntaxTest::CommonSyntaxTest(string const& _filename, langutil::EVMVersion

TestCase::TestResult CommonSyntaxTest::run(ostream& _stream, string const& _linePrefix, bool _formatted)
{
parseCustomExpectations(m_reader.stream());
parseAndAnalyze();

return conclude(_stream, _linePrefix, _formatted);
}

TestCase::TestResult CommonSyntaxTest::conclude(ostream& _stream, string const& _linePrefix, bool _formatted)
{
if (m_expectations == m_errorList)
if (expectationsMatch())
return TestResult::Success;

printExpectationAndError(_stream, _linePrefix, _formatted);
Expand All @@ -84,9 +87,9 @@ void CommonSyntaxTest::printExpectationAndError(ostream& _stream, string const&
{
string nextIndentLevel = _linePrefix + " ";
util::AnsiColorized(_stream, _formatted, {BOLD, CYAN}) << _linePrefix << "Expected result:" << endl;
printErrorList(_stream, m_expectations, nextIndentLevel, _formatted);
printExpectedResult(_stream, nextIndentLevel, _formatted);
util::AnsiColorized(_stream, _formatted, {BOLD, CYAN}) << _linePrefix << "Obtained result:" << endl;
printErrorList(_stream, m_errorList, nextIndentLevel, _formatted);
printObtainedResult(_stream, nextIndentLevel, _formatted);
}

void CommonSyntaxTest::printSource(ostream& _stream, string const& _linePrefix, bool _formatted) const
Expand Down Expand Up @@ -149,6 +152,30 @@ void CommonSyntaxTest::printSource(ostream& _stream, string const& _linePrefix,
}
}

void CommonSyntaxTest::parseCustomExpectations(istream& _stream)
{
string remainingExpectations = boost::trim_copy(readUntilEnd(_stream));
soltestAssert(
remainingExpectations.empty(),
"Found custom expectations not supported by the test case:\n" + remainingExpectations
);
}

bool CommonSyntaxTest::expectationsMatch()
{
return m_expectations == m_errorList;
}

void CommonSyntaxTest::printExpectedResult(ostream& _stream, string const& _linePrefix, bool _formatted) const
{
printErrorList(_stream, m_expectations, _linePrefix, _formatted);
}

void CommonSyntaxTest::printObtainedResult(ostream& _stream, string const& _linePrefix, bool _formatted) const
{
printErrorList(_stream, m_errorList, _linePrefix, _formatted);
}

void CommonSyntaxTest::printErrorList(
ostream& _stream,
vector<SyntaxTestError> const& _errorList,
Expand All @@ -157,7 +184,10 @@ void CommonSyntaxTest::printErrorList(
)
{
if (_errorList.empty())
util::AnsiColorized(_stream, _formatted, {BOLD, GREEN}) << _linePrefix << "Success" << endl;
{
if (_formatted)
util::AnsiColorized(_stream, _formatted, {BOLD, GREEN}) << _linePrefix << "Success" << endl;
}
else
for (auto const& error: _errorList)
{
Expand Down Expand Up @@ -194,12 +224,20 @@ string CommonSyntaxTest::errorMessage(util::Exception const& _e)

vector<SyntaxTestError> CommonSyntaxTest::parseExpectations(istream& _stream)
{
static string const customExpectationsDelimiter("// ----");

vector<SyntaxTestError> expectations;
string line;
while (getline(_stream, line))
{
auto it = line.begin();

// Anything below the delimiter is left up to the derived class to process in a custom way.
// The delimiter is optional and identical to the one that starts error expectations in
// TestCaseReader::parseSourcesAndSettingsWithLineNumber().
if (boost::algorithm::starts_with(line, customExpectationsDelimiter))
break;

skipSlashes(it, line.end());
skipWhitespace(it, line.end());

Expand Down
17 changes: 15 additions & 2 deletions test/CommonSyntaxTest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,27 @@ class CommonSyntaxTest: public frontend::test::EVMVersionRestrictedTestCase
void printSource(std::ostream& _stream, std::string const &_linePrefix = "", bool _formatted = false) const override;
void printUpdatedExpectations(std::ostream& _stream, std::string const& _linePrefix) const override
{
if (!m_errorList.empty())
printErrorList(_stream, m_errorList, _linePrefix, false);
printObtainedResult(_stream, _linePrefix, false);
}

static std::string errorMessage(util::Exception const& _e);
protected:
/// Should be implemented by those derived test cases that want to allow extra expectations
/// after the error/warning expectations. The default implementation does not allow them and
/// fails instead.
/// @param _stream Input stream positioned at the beginning of the extra expectations.
virtual void parseCustomExpectations(std::istream& _stream);

virtual void parseAndAnalyze() = 0;

/// Should return true if obtained values match expectations.
/// The default implementation only compares the error list. Derived classes that support
/// custom expectations should override this to include them in the comparison.
virtual bool expectationsMatch();

virtual void printExpectedResult(std::ostream& _stream, std::string const& _linePrefix, bool _formatted) const;
virtual void printObtainedResult(std::ostream& _stream, std::string const& _linePrefix, bool _formatted) const;

static void printErrorList(
std::ostream& _stream,
std::vector<SyntaxTestError> const& _errors,
Expand Down
8 changes: 5 additions & 3 deletions test/TestCaseReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,17 @@ pair<SourceMap, size_t> TestCaseReader::parseSourcesAndSettingsWithLineNumber(is
static string const sourceDelimiterEnd("====");
static string const comment("// ");
static string const settingsDelimiter("// ====");
static string const delimiter("// ----");
static string const expectationsDelimiter("// ----");
bool sourcePart = true;
while (getline(_stream, line))
{
lineNumber++;

if (boost::algorithm::starts_with(line, delimiter))
// Anything below the delimiter is left up to the test case to process in a custom way.
if (boost::algorithm::starts_with(line, expectationsDelimiter))
break;
else if (boost::algorithm::starts_with(line, settingsDelimiter))

if (boost::algorithm::starts_with(line, settingsDelimiter))
sourcePart = false;
else if (sourcePart)
{
Expand Down
11 changes: 10 additions & 1 deletion test/libsolidity/SyntaxTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ using namespace solidity::frontend::test;
using namespace boost::unit_test;
namespace fs = boost::filesystem;

SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion): CommonSyntaxTest(_filename, _evmVersion)
SyntaxTest::SyntaxTest(
string const& _filename,
langutil::EVMVersion _evmVersion,
Error::Severity _minSeverity
):
CommonSyntaxTest(_filename, _evmVersion),
m_minSeverity(_minSeverity)
{
m_optimiseYul = m_reader.boolSetting("optimize-yul", true);
}
Expand Down Expand Up @@ -98,6 +104,9 @@ void SyntaxTest::filterObtainedErrors()
{
for (auto const& currentError: filteredErrors())
{
if (currentError->severity() < m_minSeverity)
continue;

int locationStart = -1;
int locationEnd = -1;
string sourceName;
Expand Down
9 changes: 7 additions & 2 deletions test/libsolidity/SyntaxTest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,19 @@ class SyntaxTest: public AnalysisFramework, public solidity::test::CommonSyntaxT
{
return std::make_unique<SyntaxTest>(_config.filename, _config.evmVersion);
}
SyntaxTest(std::string const& _filename, langutil::EVMVersion _evmVersion);
SyntaxTest(
std::string const& _filename,
langutil::EVMVersion _evmVersion,
langutil::Error::Severity _minSeverity = langutil::Error::Severity::Info
);

protected:
virtual void setupCompiler();
void parseAndAnalyze() override;
virtual void filterObtainedErrors();

bool m_optimiseYul = true;
bool m_optimiseYul{};
Copy link
Collaborator

@matheusaaguiar matheusaaguiar Aug 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I see that m_optimiseYul defaults to true in the constructor, so seems harmless, but still shouldn't we keep it initialized with the same value here too?

Copy link
Member Author

@cameel cameel Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It does not really default to true. It defaults to m_reader.boolSetting("optimize-yul", true), which could end up being either. true is just as misleading here as false.

I think that using default initialization here is the least bad option. It does not show the value so it does not draw attention away from the initialization in the constructor. The actual value here is irrelevant, we only really care that it's initialized to some deterministic value.

langutil::Error::Severity m_minSeverity{};
};

}