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

[ML] Improve program counter resets between unit tests #1656

Merged
merged 2 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions include/core/CProgramCounters.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <vector>

namespace CProgramCountersTest {
class CTestFixture;
class CProgramCountersTestRunner;
struct testCounters;
struct testUnknownCounter;
Expand All @@ -28,6 +27,9 @@ struct testPersist;
}

namespace ml {
namespace test {
class CProgramCounterClearingFixture;
}
namespace counter_t {

//! The enum values must be explicitly assigned & names should have a meaningful prefix to effectively namespace counters
Expand Down Expand Up @@ -354,7 +356,7 @@ class CORE_EXPORT CProgramCounters {
const CProgramCounters& counters);

//! Befriend the test suite
friend class CProgramCountersTest::CTestFixture;
friend class test::CProgramCounterClearingFixture;
friend class CProgramCountersTest::CProgramCountersTestRunner;
friend struct CProgramCountersTest::testCounters;
friend struct CProgramCountersTest::testUnknownCounter;
Expand Down
37 changes: 37 additions & 0 deletions include/test/CProgramCounterClearingFixture.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
#ifndef INCLUDED_ml_test_CProgramCounterClearingFixture_h
#define INCLUDED_ml_test_CProgramCounterClearingFixture_h

#include <test/ImportExport.h>

namespace ml {
namespace test {

//! \brief
//! Test fixture that resets all program counters to zero
//! between tests.
//!
//! DESCRIPTION:\n
//! Program counters are implemented as a singleton object,
//! which means their values are remembered across unit tests.
//! Unit tests that care about the values of program counters
//! should use this test fixture to ensure they start from a
//! well known state. Otherwise test results can vary depending
//! on whether a test is run alone or as part of a suite.
//!
//! IMPLEMENTATION DECISIONS:\n
//! This functionality is required in multiple unit test suites,
//! hence it's in the test library.
//!
class TEST_EXPORT CProgramCounterClearingFixture {
public:
CProgramCounterClearingFixture();
};
}
}

#endif // INCLUDED_ml_test_CProgramCounterClearingFixture_h
10 changes: 5 additions & 5 deletions lib/api/unittest/CDataFrameAnalysisInstrumentationTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
#include <boost/test/tools/interface.hpp>
#include <core/CTimeUtils.h>

#include <api/CDataFrameAnalysisInstrumentation.h>

#include <test/BoostTestCloseAbsolute.h>
#include <test/CDataFrameAnalysisSpecificationFactory.h>
#include <test/CDataFrameAnalyzerTrainingFactory.h>
#include <test/CProgramCounterClearingFixture.h>

#include <rapidjson/schema.h>

Expand Down Expand Up @@ -99,7 +99,7 @@ void addOutlierTestData(TStrVec fieldNames,
}
}

BOOST_AUTO_TEST_CASE(testMemoryState) {
BOOST_FIXTURE_TEST_CASE(testMemoryState, ml::test::CProgramCounterClearingFixture) {
std::string jobId{"testJob"};
std::size_t memoryLimit{core::constants::BYTES_IN_GIGABYTES};
std::int64_t memoryUsage{500000};
Expand Down Expand Up @@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(testMemoryState) {
BOOST_TEST_REQUIRE(hasMemoryUsage);
}

BOOST_AUTO_TEST_CASE(testTrainingRegression) {
BOOST_FIXTURE_TEST_CASE(testTrainingRegression, ml::test::CProgramCounterClearingFixture) {
std::stringstream output;
auto outputWriterFactory = [&output]() {
return std::make_unique<core::CJsonOutputStreamWrapper>(output);
Expand Down Expand Up @@ -234,7 +234,7 @@ BOOST_AUTO_TEST_CASE(testTrainingRegression) {
BOOST_TEST_REQUIRE(hasMemoryUsage);
}

BOOST_AUTO_TEST_CASE(testTrainingClassification) {
BOOST_FIXTURE_TEST_CASE(testTrainingClassification, ml::test::CProgramCounterClearingFixture) {
std::stringstream output;
auto outputWriterFactory = [&output]() {
return std::make_unique<core::CJsonOutputStreamWrapper>(output);
Expand Down Expand Up @@ -298,7 +298,7 @@ BOOST_AUTO_TEST_CASE(testTrainingClassification) {
BOOST_TEST_REQUIRE(initialMemoryReport);
}

BOOST_AUTO_TEST_CASE(testOutlierDetection) {
BOOST_FIXTURE_TEST_CASE(testOutlierDetection, ml::test::CProgramCounterClearingFixture) {
std::stringstream output;
auto outputWriterFactory = [&output]() {
return std::make_unique<core::CJsonOutputStreamWrapper>(output);
Expand Down
1 change: 0 additions & 1 deletion lib/api/unittest/CInferenceModelMetadataTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
#include <boost/test/tools/interface.hpp>
#include <core/CJsonOutputStreamWrapper.h>

#include <maths/CBoostedTreeLoss.h>
Expand Down
29 changes: 9 additions & 20 deletions lib/api/unittest/CRestorePreviousStateTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <core/CJsonOutputStreamWrapper.h>
#include <core/CLogger.h>
#include <core/COsFileFuncs.h>
#include <core/CProgramCounters.h>

#include <model/CAnomalyDetectorModelConfig.h>
#include <model/CLimits.h>
Expand All @@ -19,6 +18,8 @@
#include <api/CSingleStreamSearcher.h>
#include <api/CStateRestoreStreamFilter.h>

#include <test/CProgramCounterClearingFixture.h>

#include "CTestAnomalyJob.h"
#include "CTestFieldDataCategorizer.h"

Expand All @@ -33,18 +34,6 @@

BOOST_AUTO_TEST_SUITE(CRestorePreviousStateTest)

class CTestFixture {
public:
CTestFixture() {
ml::core::CProgramCounters& counters = ml::core::CProgramCounters::instance();

// Set all counters to 0
for (std::size_t i = 0; i < ml::counter_t::NUM_COUNTERS; ++i) {
counters.counter(i) = 0;
}
}
};

namespace {

void reportPersistComplete(ml::api::CModelSnapshotJsonWriter::SModelSnapshotReport modelSnapshotReport,
Expand Down Expand Up @@ -228,7 +217,7 @@ void anomalyDetectorRestoreHelper(const std::string& stateFile,
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreDetectorBy, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreDetectorBy, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
LOG_INFO(<< "Test restoring state from version " << version.s_Version);
anomalyDetectorRestoreHelper(
Expand All @@ -237,7 +226,7 @@ BOOST_FIXTURE_TEST_CASE(testRestoreDetectorBy, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreDetectorOver, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreDetectorOver, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
LOG_INFO(<< "Test restoring state from version " << version.s_Version);
anomalyDetectorRestoreHelper("testfiles/state/" + version.s_Version + "/over_detector_state.json",
Expand All @@ -246,7 +235,7 @@ BOOST_FIXTURE_TEST_CASE(testRestoreDetectorOver, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreDetectorPartition, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreDetectorPartition, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
LOG_INFO(<< "Test restoring state from version " << version.s_Version);
anomalyDetectorRestoreHelper("testfiles/state/" + version.s_Version + "/partition_detector_state.json",
Expand All @@ -255,7 +244,7 @@ BOOST_FIXTURE_TEST_CASE(testRestoreDetectorPartition, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreDetectorDc, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreDetectorDc, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
LOG_INFO(<< "Test restoring state from version " << version.s_Version);
anomalyDetectorRestoreHelper(
Expand All @@ -264,7 +253,7 @@ BOOST_FIXTURE_TEST_CASE(testRestoreDetectorDc, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreDetectorCount, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreDetectorCount, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
LOG_INFO(<< "Test restoring state from version " << version.s_Version);
anomalyDetectorRestoreHelper("testfiles/state/" + version.s_Version + "/count_detector_state.json",
Expand All @@ -273,7 +262,7 @@ BOOST_FIXTURE_TEST_CASE(testRestoreDetectorCount, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreNormalizer, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreNormalizer, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
ml::model::CAnomalyDetectorModelConfig modelConfig =
ml::model::CAnomalyDetectorModelConfig::defaultConfig(3600);
Expand All @@ -284,7 +273,7 @@ BOOST_FIXTURE_TEST_CASE(testRestoreNormalizer, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testRestoreCategorizer, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testRestoreCategorizer, ml::test::CProgramCounterClearingFixture) {
for (const auto& version : BWC_VERSIONS) {
LOG_INFO(<< "Test restoring state from version " << version.s_Version);
categorizerRestoreHelper("testfiles/state/" + version.s_Version + "/categorizer_state.json",
Expand Down
28 changes: 7 additions & 21 deletions lib/core/unittest/CProgramCountersTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <core/CRegex.h>
#include <core/CThread.h>

#include <test/CProgramCounterClearingFixture.h>
#include <test/CRandomNumbers.h>

#include <boost/test/unit_test.hpp>
Expand All @@ -24,21 +25,6 @@ BOOST_AUTO_TEST_SUITE(CProgramCountersTest)

const int TEST_COUNTER{0u};

class CTestFixture {
public:
CTestFixture() {
ml::core::CProgramCounters& counters = ml::core::CProgramCounters::instance();

// Set all counters to 0
for (std::size_t i = 0; i < ml::counter_t::NUM_COUNTERS; ++i) {
counters.counter(i) = 0;
}

// Clear the cache
counters.m_Cache.clear();
}
};

class CProgramCountersTestRunner : public ml::core::CThread {
public:
static const int N = 6;
Expand Down Expand Up @@ -89,7 +75,7 @@ void restore(const std::string& staticsXml) {
&ml::core::CProgramCounters::staticsAcceptRestoreTraverser));
}

BOOST_FIXTURE_TEST_CASE(testCounters, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testCounters, ml::test::CProgramCounterClearingFixture) {
ml::core::CProgramCounters& counters = ml::core::CProgramCounters::instance();

using TCounter = ml::core::CProgramCounters::TCounter;
Expand Down Expand Up @@ -130,7 +116,7 @@ BOOST_FIXTURE_TEST_CASE(testCounters, CTestFixture) {
BOOST_REQUIRE_EQUAL(TCounter(0x1000000), counters.counter(TEST_COUNTER));
}

BOOST_FIXTURE_TEST_CASE(testPersist, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testPersist, ml::test::CProgramCounterClearingFixture) {
// Run the first set of checks without registering a specific subset of counters
// in order to test the entire set now and in the future
using TCounter = ml::core::CProgramCounters::TCounter;
Expand Down Expand Up @@ -278,7 +264,7 @@ BOOST_FIXTURE_TEST_CASE(testPersist, CTestFixture) {
BOOST_REQUIRE_EQUAL(outputOrder1, outputOrder2);
}

BOOST_FIXTURE_TEST_CASE(testUnknownCounter, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testUnknownCounter, ml::test::CProgramCounterClearingFixture) {
ml::core::CProgramCounters& counters = ml::core::CProgramCounters::instance();

// Name of the log file to use. It must match the name specified
Expand Down Expand Up @@ -309,7 +295,7 @@ BOOST_FIXTURE_TEST_CASE(testUnknownCounter, CTestFixture) {
std::remove(logFile);
}

BOOST_FIXTURE_TEST_CASE(testMissingCounter, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testMissingCounter, ml::test::CProgramCounterClearingFixture) {
// explicitly register interest in a particular set of counters
const ml::counter_t::TCounterTypeSet counterSet{
ml::counter_t::E_TSADNumberNewPeopleNotAllowed,
Expand Down Expand Up @@ -348,7 +334,7 @@ BOOST_FIXTURE_TEST_CASE(testMissingCounter, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testCacheCounters, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testCacheCounters, ml::test::CProgramCounterClearingFixture) {
ml::core::CProgramCounters& counters = ml::core::CProgramCounters::instance();

// confirm that initially the cache is empty
Expand Down Expand Up @@ -386,7 +372,7 @@ BOOST_FIXTURE_TEST_CASE(testCacheCounters, CTestFixture) {
}
}

BOOST_FIXTURE_TEST_CASE(testMax, CTestFixture) {
BOOST_FIXTURE_TEST_CASE(testMax, ml::test::CProgramCounterClearingFixture) {
ml::test::CRandomNumbers rng;
std::size_t m1{0}, m2{0};
std::thread thread1{[&m1, &rng] {
Expand Down
26 changes: 26 additions & 0 deletions lib/test/CProgramCounterClearingFixture.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

#include <test/CProgramCounterClearingFixture.h>

#include <core/CProgramCounters.h>

namespace ml {
namespace test {

CProgramCounterClearingFixture::CProgramCounterClearingFixture() {
core::CProgramCounters& counters{core::CProgramCounters::instance()};

// Set all counters to 0
for (std::size_t i = 0; i < counter_t::NUM_COUNTERS; ++i) {
counters.counter(i) = 0;
}

// Clear the cache
counters.m_Cache.clear();
}
}
}
1 change: 1 addition & 0 deletions lib/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ SRCS= \
CDataFrameAnalyzerTrainingFactory.cc \
CMultiFileDataAdder.cc \
CMultiFileSearcher.cc \
CProgramCounterClearingFixture.cc \
CRandomNumbers.cc \
CTestObserver.cc \
CTestTmpDir.cc \
Expand Down