Skip to content

Commit

Permalink
Remove draconian regex filter that eliminates entire tests from being…
Browse files Browse the repository at this point in the history
… run (#246)

Resolves #238 

TODOs:
- [x] add a new macro to detect if running in CI
- [x] enable the skipping of some tests that don't work in the CI env

Authors:
  - Ryan Olson (https://github.com/ryanolson)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: #246
  • Loading branch information
ryanolson authored Dec 9, 2022
1 parent c4f1c0d commit 83f6499
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci_pipe.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ jobs:
username: '$oauthtoken'
password: ${{ secrets.NGC_API_KEY }}
image: ${{ inputs.test_container }}
options: --cap-add=sys_nice
options: "--cap-add=sys_nice --cap-add=sys_ptrace"
env:
NVIDIA_VISIBLE_DEVICES: ${{ env.NVIDIA_VISIBLE_DEVICES }}
PARALLEL_LEVEL: '10'
Expand Down
5 changes: 0 additions & 5 deletions ci/scripts/github/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,7 @@ cmake -B build -G Ninja ${CMAKE_BUILD_ALL_FEATURES} .
rapids-logger "Running C++ Tests"
cd ${MRC_ROOT}/build
set +e
# Tests known to be failing
# Issues:
# * test_mrc_private - https://github.com/nv-morpheus/MRC/issues/33
# * nvrpc - https://github.com/nv-morpheus/MRC/issues/34
ctest --output-on-failure \
--exclude-regex "test_mrc_private|nvrpc" \
--output-junit ${REPORTS_DIR}/report_ctest.xml

CTEST_RESULTS=$?
Expand Down
2 changes: 0 additions & 2 deletions ci/scripts/github/test_codecov.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ set +e
# Tests known to be failing
# Issues:
# * test_mrc_private - https://github.com/nv-morpheus/MRC/issues/33
# * nvrpc - https://github.com/nv-morpheus/MRC/issues/34
ctest --output-on-failure \
--exclude-regex "test_mrc_private|nvrpc" \
--output-junit ${REPORTS_DIR}/report_ctest.xml

CTEST_RESULTS=$?
Expand Down
4 changes: 4 additions & 0 deletions cpp/mrc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ target_compile_definitions(libmrc
$<$<BOOL:${MRC_BUILD_BENCHMARKS}>:MRC_ENABLE_BENCHMARKING>
)

if (MRC_ENABLE_CODECOV)
target_compile_definitions(libmrc INTERFACE "MRC_CODECOV_ENABLED")
endif()

target_compile_features(libmrc PUBLIC cxx_std_20)

set_target_properties(libmrc PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
Expand Down
4 changes: 2 additions & 2 deletions cpp/mrc/src/internal/control_plane/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ void Server::drop_stream(const stream_id_t& stream_id)
auto search = m_connections.streams().find(stream_id);
if (search == m_connections.streams().end())
{
LOG(WARNING) << "attempting to drop stream_id: " << stream_id
<< " which is not found in set of connected streams";
LOG(FATAL) << "attempting to drop stream_id: " << stream_id
<< " which is not found in set of connected streams";
}

auto writer = search->second->writer();
Expand Down
10 changes: 10 additions & 0 deletions cpp/mrc/src/tests/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,20 @@

#pragma once

#include <gtest/gtest.h>

#include <cstdlib>
#include <functional>
#include <memory>

namespace mrc {

#ifdef MRC_CODECOV_ENABLED
#define SKIP_IF_CODE_COV() GTEST_SKIP() << "Skipping test when code coverage is enabled";
#else
#define SKIP_IF_CODE_COV()
#endif

class Options;
} // namespace mrc
namespace mrc::internal::system {
Expand Down
2 changes: 1 addition & 1 deletion cpp/mrc/src/tests/test_expected.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ TEST_F(TestExpected, Chaining3)

EXPECT_FALSE(rc);
EXPECT_EQ(rc.error().message(), "void fail");
EXPECT_ANY_THROW(rc->value());
EXPECT_ANY_THROW(rc.value());
}

TEST_F(TestExpected, UniquePointer)
Expand Down
11 changes: 11 additions & 0 deletions cpp/mrc/src/tests/test_ucx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

#include "common.hpp"

#include "internal/ucx/all.hpp"
#include "internal/ucx/endpoint.hpp"

Expand Down Expand Up @@ -72,6 +74,9 @@ TEST_F(TestUCX, CreateWorkerAddress)

TEST_F(TestUCX, EndpointsInProcess)
{
// note this test really should use a progress engine
GTEST_SKIP();

auto worker_1 = std::make_shared<Worker>(m_context);
auto worker_2 = std::make_shared<Worker>(m_context);

Expand Down Expand Up @@ -107,6 +112,8 @@ static void rdma_get_callback(void* request, ucs_status_t status, void* user_dat

TEST_F(TestUCX, Get)
{
SKIP_IF_CODE_COV()

auto context = std::make_shared<Context>();

auto worker_get_src = std::make_shared<Worker>(context);
Expand Down Expand Up @@ -205,6 +212,8 @@ class SendRecvManager

TEST_F(TestUCX, Recv)
{
SKIP_IF_CODE_COV();

auto context = std::make_shared<Context>();

auto worker_src = std::make_shared<Worker>(context);
Expand Down Expand Up @@ -279,6 +288,8 @@ TEST_F(TestUCX, Recv)

TEST_F(TestUCX, Recv2)
{
SKIP_IF_CODE_COV();

auto context = std::make_shared<Context>();

auto worker_src = std::make_shared<Worker>(context);
Expand Down
8 changes: 8 additions & 0 deletions cpp/mrc/tests/test_mrc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,21 @@
#include <chrono>
#include <condition_variable>
#include <cstddef>
#include <cstdlib>
#include <mutex> // for mutex & unique_lock

#define TEST_CLASS(name) \
class Test##name : public ::testing::Test \
{}

namespace mrc {

#ifdef MRC_CODECOV_ENABLED
#define SKIP_IF_CODE_COV() GTEST_SKIP() << "Skipping test when code coverage is enabled";
#else
#define SKIP_IF_CODE_COV()
#endif

// class that records when it's moved/copied
struct CopyMoveCounter
{
Expand Down

0 comments on commit 83f6499

Please sign in to comment.