Skip to content

Commit

Permalink
Fixed a tracing test bug
Browse files Browse the repository at this point in the history
The thread sometimes can start after the app has stopped. So in setup
test we need to make sure the thread started.
  • Loading branch information
shuhaowu committed Jul 19, 2024
1 parent 827f9ac commit 5015289
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
3 changes: 3 additions & 0 deletions tests/tracing/helpers/mock_threads.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "mock_threads.h"

#include <atomic>

#include "utils.h"

using namespace std::chrono_literals;
Expand Down Expand Up @@ -38,6 +40,7 @@ void MockRegularThread::RunOneIteration(std::function<void(MockRegularThread*)>&
}

void MockRegularThread::Run() {
started_ = true;
while (!StopRequested()) {
{
std::unique_lock lock(mutex_);
Expand Down
5 changes: 5 additions & 0 deletions tests/tracing/helpers/mock_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ extern const char* kCyclicThreadName;
class MockRegularThread : public cactus_rt::Thread {
static cactus_rt::ThreadConfig MakeConfig();

std::atomic_bool started_ = false;
std::condition_variable cv_;
std::mutex mutex_;
std::optional<std::function<void(MockRegularThread*)>> f_ = std::nullopt;
Expand All @@ -28,6 +29,10 @@ class MockRegularThread : public cactus_rt::Thread {
return Tracer();
}

inline bool Started() const noexcept {
return started_;
}

protected:
void Run() final;
};
Expand Down
12 changes: 9 additions & 3 deletions tests/tracing/single_threaded_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
#include <gtest/gtest.h>
#include <quill/detail/LogManager.h>

#include <chrono>
#include <memory>
#include <thread>

#include "helpers/assert_helpers.h"
#include "helpers/mock_sink.h"
Expand Down Expand Up @@ -37,6 +39,9 @@ class SingleThreadTracingTest : public ::testing::Test {
app_.RegisterThread(regular_thread_);
app_.StartTraceSession(sink_); // TODO: make each test manually start the trace session!
app_.Start();
while (!regular_thread_->Started()) {
std::this_thread::sleep_for(std::chrono::milliseconds(1)); // Not that efficient but OK
}
}

void TearDown() override {
Expand Down Expand Up @@ -266,9 +271,12 @@ TEST_F(SingleThreadTracingTest, StopTracingAndNoEventsAreRecorded) {

auto traces = sink_->LoggedTraces();
auto packets = GetPacketsFromTraces(traces);
ASSERT_EQ(packets.size(), 1);
ASSERT_EQ(packets.size(), 2);

AssertIsProcessTrackDescriptor(*packets[0], kAppName);
const auto process_track_uuid = packets[0]->track_descriptor().uuid();

AssertIsThreadTrackDescriptor(*packets[1], kRegularThreadName, process_track_uuid);
}

TEST_F(SingleThreadTracingTest, RestartTracingStartsNewSession) {
Expand Down Expand Up @@ -310,8 +318,6 @@ TEST_F(SingleThreadTracingTest, RestartTracingStartsNewSession) {
AssertIsThreadTrackDescriptor(*packets2[1], kRegularThreadName, process_track_uuid);
auto thread_track_uuid = packets2[1]->track_descriptor().uuid();

std::cout << "packets2: " << packets2[2]->ShortDebugString() << "\n";

// Event1 is emitted as interned data because that thread is still active and the event name got interned previously.
auto event_names = GetInternedEventNames(*packets2[2]);
ASSERT_EQ(event_names.size(), 1);
Expand Down

0 comments on commit 5015289

Please sign in to comment.