Skip to content

Add Integration Test for Remote Logging - #344

Open
KonradBreitsprecherBkd wants to merge 1 commit into
mainfrom
dev/ITest-logging
Open

Add Integration Test for Remote Logging#344
KonradBreitsprecherBkd wants to merge 1 commit into
mainfrom
dev/ITest-logging

Conversation

@KonradBreitsprecherBkd

Copy link
Copy Markdown
Contributor

Summary

Add a new integration test for remote logging with three participants in SilKit/IntegrationTests/ITest_RemoteLogging.cpp.

  • Creates three participants:
    • Sender1 and Sender2 configured with a remote logging sink
    • Receiver configured with LogFromRemotes: true and a file sink
  • Sender1 and Sender2 each emit a unique log message
  • Verifies that the receiver-side log file contains both messages
  • Timeout-based polling for file creation and content visibility
  • Added scoped cleanup to remove generated log files after test execution (including failure paths)

Signed-off-by: Konrad Breitsprecher <Konrad.Breitsprecher@vector.com>
@KonradBreitsprecherBkd KonradBreitsprecherBkd added the needs reviewer This issue is looking for a reviewer. label Jun 15, 2026
@snps-fiodorov
snps-fiodorov requested review from snps-behrens and removed request for snps-fiodorov July 29, 2026 09:24
@snps-fiodorov snps-fiodorov removed the needs reviewer This issue is looking for a reviewer. label Jul 29, 2026

@snps-behrens snps-behrens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks pretty good already. I left some comments and questions.

Comment thread SilKit/IntegrationTests/ITest_RemoteLogging.cpp
Comment thread SilKit/IntegrationTests/ITest_RemoteLogging.cpp
" LogName: "
+ receiverLogName + "\n";

SimTestHarness testHarness{{"Sender1", "Sender2", "Receiver"}, "silkit://localhost:0", true};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This version of the constructor is marked deprecated, and says the single-argument version taking a SimTestHarnessArgs object should be used.

Level: Trace
)";

const auto receiverConfig = "Logging:\n"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this could be made more consistent with the above senderConfig by using a raw string inside a call to fmt::format().

std::filesystem::path logFile;
std::string logContent;

while (std::chrono::steady_clock::now() < deadline)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this polling loop necessary?

const std::string sender1Message = "remote-log-from-sender-1";
const std::string sender2Message = "remote-log-from-sender-2";

std::atomic<bool> sender1Logged{false};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think these std::atomics are needed, right?

auto* sender2TimeSync = sender2->GetOrCreateTimeSyncService();
auto* receiverTimeSync = receiver->GetOrCreateTimeSyncService();

auto* sender1Logger = sender1->Participant()->GetLogger();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like SimParticipant also has a GetLogger function. Maybe use that here instead?

}
}

if (candidates.size() != 1u)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to distiguish between the two failure modes here already? candidates.emty() is an expected result during polling, but candidates.size() > 1 would indicate somehting else went wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants