Conversation
Codecov Report
@@ Coverage Diff @@
## master #5801 +/- ##
=========================================
Coverage ? 64.07%
=========================================
Files ? 359
Lines ? 30810
Branches ? 3417
=========================================
Hits ? 19742
Misses ? 9845
Partials ? 1223 |
| bcGeneralTestsFixture() : StateTestFixtureBase({TestExecution::RequireOptionAll}) {} | ||
| }; | ||
|
|
||
| template <class T> |
There was a problem hiding this comment.
I think you can easy get rid of duplication here like this
template <class T>
class BlockChainTestFixture
{
public:
BlockChainTestFixture(std::set<TestExecution> const& _execFlags = {}, std::vector<std::string> const& _timeConsumingCases)
{
T suite;
if (_execFlags.count(TestExecution::NotRefillable) &&
(Options::get().fillchain || Options::get().filltests))
BOOST_FAIL("Tests are sealed and not refillable!");
string const casename = boost::unit_test::framework::current_test_case().p_name;
boost::filesystem::path const suiteFillerPath = suite.getFullPathFiller(casename).parent_path();
// skip time consuming tests, unless --all option is given
if (!test::Options::get().all && contains(_timeConsumingCases, casename))
{
cnote << "Skipping " << casename << " because --all option is not specified.\n";
test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
return;
}
suite.runAllTestsInFolder(casename);
test::TestOutputHelper::get().markTestFolderAsFinished(suiteFillerPath, casename);
}
};
class BlockChainValidBlocksTestFixture
: public BlockChainTestFixture<BlockchainValidTestSuite>
{
public:
BlockChainValidBlocksTestFixture()
: BlockChainValidBlocksTestFixture({TestExecution::NotRefillable}, {"bcWalletTest"})
{}
};
There was a problem hiding this comment.
Actually, if stWallet exists in only in either valid or invalid suite, you can use the exact same code with stWallet check for both suites (it won't skip stWallet if it doesn't exist there)
There was a problem hiding this comment.
I think it is ok to use just one fixture class in this case
|
Does this change anything regarding how to run the tests? (command line) |
to run the old blockchain tests now use command: |
gumb0
left a comment
There was a problem hiding this comment.
Looks good, need to add changelog item
"testeth -t BlockchainTests command now doesn't run the tests for the forks before Istanbul. To run those tests use a separate LegacyTests suite with command testeth -t LegacyTests/Constantinople/BlockchainTests"
check the commands^
|
updated the change log. |
gumb0
left a comment
There was a problem hiding this comment.
Don't merge, please rebase on master instead
|
the errors seems to be in libp2p test again. merge? |
test repo update is in progress