From cbd9dc1923aa4d2d4c72f59362853a8b07af661c Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:42:06 +0700 Subject: [PATCH 01/18] Add QtTest unit test harness with protocol seed suite Introduces src/test/, a QtTest-based unit test target (jamulus-test) covering CProtocol's on-wire framing contract: - golden-frame tests pin the exact bytes production emits today for a fixed-length and a variable-length message body, so any accidental wire format change fails loudly instead of silently -- the expected hex string is built field by field (TAG, message ID, sequence counter, data length, data, CRC), each `+=` line commented with what that field is, against a fresh CProtocolTester's SentFrames() - a frame acceptance/rejection contract test trio (AcceptValidFrame, RejectInvalidFrame with bad CRC/length/truncation/junk rows, and IgnoreAcknWithEmptyBody -- the regression test for https://github.com/jamulussoftware/jamulus/issues/302, fixed in 024ebb47: an ACKN message with a valid checksum but no data caused an out-of-bounds read; the crafted frame is still well-formed so it's accepted at the frame level, but must be silently dropped with no signal fired, and the ASan/UBSan matrix job is what gives the "no OOB" part of that its teeth) checks how many frames the receiving side actually parsed and accepted, building each frame imperatively from a real sent frame (LastSentFrame()) plus a small set of named mutation helpers - one round-trip test through a connected sender/receiver pair (CProtocolTester) sends a message on one side and asserts, against an event log of everything the other side received, that exactly the expected signal fired with the expected arguments CProtocolTester (src/test/protocoltester.h) is the one public type this header exposes: a struct-like pair of CProtocol instances (Sender, Receiver) wired together in both directions -- the same wiring CChannel uses for two peers, including routing acknowledgements back so the sender's queue advances -- plus: - a sent frame log: every frame Sender hands to MessReadyForSending is recorded as both a hex string (SentFrames(), for golden frame comparisons) and raw bytes (LastSentFrame(), for tests that mutate a real frame); a fresh instance's first send has sequence counter 0, which is what makes golden frames byte-for-byte reproducible - a receiver-side acceptance count: ReceivedAndAcceptedMessageCount() counts frames that passed frame parsing on the receiver side and were handed to ParseMessageBody(); a failed parse -- whether from a real send or a malformed frame injected via SendRawBytes() -- is a silently dropped, countable non-event rather than an assertion failure, and is tracked per direction so an ACK flowing back to the sender never affects the receiver's own count - a received signal log: every non-CLM "receiving" signal CProtocol emits from ParseMessageBody (protocol.h's ChangeJittBufSize..RecorderStateReceived block, 21 signals) is wired to append one formatted "SignalName(arg1, arg2)" line to ReceivedLog(), so a test can QCOMPARE the whole log against what it expects -- which also proves, for free, that no other wired signal fired - ToByteArray()/FromByteArray(), the frame mutators TruncateBy()/CorruptCRC()/SetDeclaredLength(), and ReplaceIdAndBody() (rebuilds a frame with a different ID/body, recomputing length and CRC) for crafting ID/body combinations a real Create*Mes() call can't produce Tests hold their own frames/expectations and call these helpers directly, with no builder chain, lambda-taking capture function, or shared error-message plumbing in the public surface to look through. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .gitignore | 3 + src/test/protocoltester.h | 288 ++++++++++++++++++++++++++++++++++++++ src/test/test.pro | 53 +++++++ src/test/tst_protocol.cpp | 194 +++++++++++++++++++++++++ 4 files changed, 538 insertions(+) create mode 100644 src/test/protocoltester.h create mode 100644 src/test/test.pro create mode 100644 src/test/tst_protocol.cpp diff --git a/.gitignore b/.gitignore index 3b23a7cd6d..95ef8d0b1e 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,9 @@ build/ deploy/ build-gui/ build-nox/ +build-test/ +test-output.txt +test-results.xml jamulus.sln jamulus.vcxproj jamulus.vcxproj.filters diff --git a/src/test/protocoltester.h b/src/test/protocoltester.h new file mode 100644 index 0000000000..f73ca408f4 --- /dev/null +++ b/src/test/protocoltester.h @@ -0,0 +1,288 @@ +/******************************************************************************\ + * Copyright (c) 2026 + * + * Author(s): + * The Jamulus Development Team + * + * Licensed under AGPL 3.0 or any later version. See COPYING for details. + * +\******************************************************************************/ + +#pragma once + +#include +#include "protocol.h" + +/* CProtocolTester **************************************************************/ + +// Wires two CProtocol instances together in both directions -- what CChannel +// does with two peers -- so a test can send on one side and observe what the +// other received. Both directions are wired so ACKs also flow back and +// advance CProtocol's own send queue. +class CProtocolTester +{ +public: + CProtocolTester() + { + Connect ( Sender, Receiver, iReceiverAcceptedCount ); + Connect ( Receiver, Sender, iSenderAcceptedCount ); + WireSentFrameLog(); + WireReceivedLog(); + } + + CProtocol Sender; + CProtocol Receiver; + + /* sent frame log ----------------------------------------------------- + * Every frame Sender emits is recorded as hex (SentFrames()) and raw + * bytes (LastSentFrame()). Determinism: a fresh instance's first send + * has cnt == 0, so golden checks must run first on a fresh Tester. + */ + + QStringList SentFrames() const { return strlSentFrames; } + + CVector LastSentFrame() const { return vecbyLastSentFrame; } + + /* receiver-side acceptance ---------------------------------------------- */ + + // Frames that passed frame parsing on the RECEIVER side and were handed + // to ParseMessageBody(); a failed parse (real or injected) doesn't count. + int ReceivedAndAcceptedMessageCount() const { return iReceiverAcceptedCount; } + + // Feeds raw bytes into the Receiver's parse path exactly as a frame sent + // over the wire would be, for crafted or malformed inputs a real + // Create*Mes() call can't produce. + void SendRawBytes ( const QByteArray& baFrame ) { deliver ( FromByteArray ( baFrame ), Receiver, iReceiverAcceptedCount ); } + + /* received signal log -------------------------------------------------- + * Every "receiving" signal CProtocol emits from ParseMessageBody is + * logged here as "SignalName(arg1, arg2)" -- QCOMPARE the whole list to + * also prove no *other* signal fired. + */ + + QStringList ReceivedLog() const { return strlReceivedLog; } + + void ClearReceivedLog() { strlReceivedLog.clear(); } + + // 4 decimals is the comparison precision -- coarser than the 1/32768 wire quantization step. + static QString FormatFloat ( const float f ) { return QString::number ( f, 'f', 4 ); } + + /* byte conversion ----------------------------------------------------- */ + + static QByteArray ToByteArray ( const CVector& vecbyData ) + { + return QByteArray ( reinterpret_cast ( vecbyData.data() ), vecbyData.Size() ); + } + + static CVector FromByteArray ( const QByteArray& baData ) + { + const int iSize = static_cast ( baData.size() ); + + CVector vecbyData ( iSize ); + + for ( int i = 0; i < iSize; i++ ) + { + vecbyData[i] = static_cast ( baData[i] ); + } + + return vecbyData; + } + + /* frame mutation helpers ----------------------------------------------- + * Operate in place on a frame the test holds, for crafting rows/inputs + * that a real Create*Mes() call can't produce. + */ + + // chops iBytes off the end of vecbyFrame, e.g. to cut into the CRC or the + // body + static void TruncateBy ( CVector& vecbyFrame, const int iBytes ) { vecbyFrame.resize ( vecbyFrame.Size() - iBytes ); } + + // flips every bit of the trailing CRC byte so the checksum no longer + // matches the header plus body + static void CorruptCRC ( CVector& vecbyFrame ) + { + vecbyFrame[vecbyFrame.Size() - 1] = static_cast ( vecbyFrame[vecbyFrame.Size() - 1] ^ 0xFF ); + } + + // overwrites vecbyFrame's declared body length field, independently of the + // body bytes actually present + static void SetDeclaredLength ( CVector& vecbyFrame, const int iLen ) + { + vecbyFrame[5] = static_cast ( iLen & 0xFF ); + vecbyFrame[6] = static_cast ( ( iLen >> 8 ) & 0xFF ); + } + + // Rebuilds vecbyFrame with a different message ID and body, recomputing + // the declared length and CRC to match -- for crafting a well-formed + // frame combination a real Create*Mes() call can't produce (e.g. ACKN + // with no body). + static void ReplaceIdAndBody ( CVector& vecbyFrame, const int iID, const CVector& vecbyNewBody ) + { + const int iBodyLen = vecbyNewBody.Size(); + CVector vecbyNewFrame ( MESS_LEN_WITHOUT_DATA_BYTE + iBodyLen ); + + vecbyNewFrame[0] = 0; // TAG + vecbyNewFrame[1] = 0; + vecbyNewFrame[2] = static_cast ( iID & 0xFF ); // message ID + vecbyNewFrame[3] = static_cast ( ( iID >> 8 ) & 0xFF ); + vecbyNewFrame[4] = 0; // sequence counter + vecbyNewFrame[5] = static_cast ( iBodyLen & 0xFF ); // data length + vecbyNewFrame[6] = static_cast ( ( iBodyLen >> 8 ) & 0xFF ); + + for ( int i = 0; i < iBodyLen; i++ ) + { + vecbyNewFrame[MESS_HEADER_LENGTH_BYTE + i] = vecbyNewBody[i]; + } + + CCRC CRCObj; + for ( int i = 0; i < MESS_HEADER_LENGTH_BYTE + iBodyLen; i++ ) + { + CRCObj.AddByte ( vecbyNewFrame[i] ); + } + + const uint32_t iCRC = CRCObj.GetCRC(); + const int iCRCPos = MESS_HEADER_LENGTH_BYTE + iBodyLen; + + vecbyNewFrame[iCRCPos] = static_cast ( iCRC & 0xFF ); + vecbyNewFrame[iCRCPos + 1] = static_cast ( ( iCRC >> 8 ) & 0xFF ); + + vecbyFrame = vecbyNewFrame; + } + +private: + // note that CProtocol::ParseMessageFrame() returns true on error, this + // helper returns true on success to make the rest of this file easier to + // read + static bool ParseFrame ( const CVector& vecbyFrame, CVector& vecbyMesBodyData, int& iRecCounter, int& iRecID ) + { + return !CProtocol::ParseMessageFrame ( vecbyFrame, vecbyFrame.Size(), vecbyMesBodyData, iRecCounter, iRecID ); + } + + // Parses vecMessage and, if valid, hands it to To.ParseMessageBody(), + // counting it as accepted; an invalid frame is silently dropped -- a + // countable non-event rather than a test failure. + void deliver ( const CVector& vecMessage, CProtocol& To, int& iAcceptedCount ) + { + CVector vecbyMesBodyData; + int iRecCounter = 0; + int iRecID = 0; + + if ( ParseFrame ( vecMessage, vecbyMesBodyData, iRecCounter, iRecID ) ) + { + To.ParseMessageBody ( vecbyMesBodyData, iRecCounter, iRecID ); + iAcceptedCount++; + } + } + + // Delivers every frame emitted by From to To via deliver() above, + // emulating what CChannel does with received network packets, and counts + // it under iAcceptedCount (a separate counter per direction, so Sender's + // ACKs -- delivered Receiver -> Sender -- never affect Receiver's count). + void Connect ( CProtocol& From, CProtocol& To, int& iAcceptedCount ) + { + QObject::connect ( &From, &CProtocol::MessReadyForSending, &To, [this, &To, &iAcceptedCount] ( CVector vecMessage ) { + deliver ( vecMessage, To, iAcceptedCount ); + } ); + } + + // See the "sent frame log" section above. + void WireSentFrameLog() + { + QObject::connect ( &Sender, &CProtocol::MessReadyForSending, [this] ( CVector vecMessage ) { + vecbyLastSentFrame = vecMessage; + strlSentFrames << QString::fromLatin1 ( ToByteArray ( vecMessage ).toHex ( ' ' ) ); + } ); + } + + // One connect + one format line per signal, in protocol.h's declaration + // order. ConClientListMesReceived/ChangeChanInfo log just enough to + // identify what arrived (a count, a name), not full field detail. + void WireReceivedLog() + { + QObject::connect ( &Receiver, &CProtocol::ChangeJittBufSize, [this] ( int iNewJitBufSize ) { + strlReceivedLog << QStringLiteral ( "ChangeJittBufSize(%1)" ).arg ( iNewJitBufSize ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ReqJittBufSize, [this]() { strlReceivedLog << QStringLiteral ( "ReqJittBufSize()" ); } ); + + QObject::connect ( &Receiver, &CProtocol::ChangeNetwBlSiFact, [this] ( int iNewNetwBlSiFact ) { + strlReceivedLog << QStringLiteral ( "ChangeNetwBlSiFact(%1)" ).arg ( iNewNetwBlSiFact ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ClientIDReceived, [this] ( int iChanID ) { + strlReceivedLog << QStringLiteral ( "ClientIDReceived(%1)" ).arg ( iChanID ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ChangeChanGain, [this] ( int iChanID, float fNewGain ) { + strlReceivedLog << QStringLiteral ( "ChangeChanGain(%1, %2)" ).arg ( iChanID ).arg ( FormatFloat ( fNewGain ) ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ChangeChanPan, [this] ( int iChanID, float fNewPan ) { + strlReceivedLog << QStringLiteral ( "ChangeChanPan(%1, %2)" ).arg ( iChanID ).arg ( FormatFloat ( fNewPan ) ); + } ); + + QObject::connect ( &Receiver, &CProtocol::MuteStateHasChangedReceived, [this] ( int iCurID, bool bIsMuted ) { + strlReceivedLog << QStringLiteral ( "MuteStateHasChangedReceived(%1, %2)" ).arg ( iCurID ).arg ( bIsMuted ? "true" : "false" ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ConClientListMesReceived, [this] ( CVector vecChanInfo ) { + strlReceivedLog << QStringLiteral ( "ConClientListMesReceived(%1 channel(s))" ).arg ( vecChanInfo.Size() ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ServerFullMesReceived, [this]() { + strlReceivedLog << QStringLiteral ( "ServerFullMesReceived()" ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ReqConnClientsList, [this]() { strlReceivedLog << QStringLiteral ( "ReqConnClientsList()" ); } ); + + QObject::connect ( &Receiver, &CProtocol::ChangeChanInfo, [this] ( CChannelCoreInfo ChanInfo ) { + strlReceivedLog << QStringLiteral ( "ChangeChanInfo(%1)" ).arg ( ChanInfo.strName ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ReqChanInfo, [this]() { strlReceivedLog << QStringLiteral ( "ReqChanInfo()" ); } ); + + QObject::connect ( &Receiver, &CProtocol::ChatTextReceived, [this] ( QString strChatText ) { + strlReceivedLog << QStringLiteral ( "ChatTextReceived(%1)" ).arg ( strChatText ); + } ); + + QObject::connect ( &Receiver, &CProtocol::NetTranspPropsReceived, [this] ( CNetworkTransportProps NetworkTransportProps ) { + strlReceivedLog << QStringLiteral ( "NetTranspPropsReceived(%1, %2, %3, %4, %5, %6, %7)" ) + .arg ( NetworkTransportProps.iBaseNetworkPacketSize ) + .arg ( NetworkTransportProps.iBlockSizeFact ) + .arg ( NetworkTransportProps.iNumAudioChannels ) + .arg ( NetworkTransportProps.iSampleRate ) + .arg ( static_cast ( NetworkTransportProps.eAudioCodingType ) ) + .arg ( static_cast ( NetworkTransportProps.eFlags ) ) + .arg ( NetworkTransportProps.iAudioCodingArg ); + } ); + + QObject::connect ( &Receiver, &CProtocol::ReqNetTranspProps, [this]() { strlReceivedLog << QStringLiteral ( "ReqNetTranspProps()" ); } ); + + QObject::connect ( &Receiver, &CProtocol::ReqSplitMessSupport, [this]() { strlReceivedLog << QStringLiteral ( "ReqSplitMessSupport()" ); } ); + + QObject::connect ( &Receiver, &CProtocol::SplitMessSupported, [this]() { strlReceivedLog << QStringLiteral ( "SplitMessSupported()" ); } ); + + QObject::connect ( &Receiver, &CProtocol::RawAudioSupported, [this]() { strlReceivedLog << QStringLiteral ( "RawAudioSupported()" ); } ); + + // ELicenceType/ERecorderState are unregistered enums -- a lambda + // connect sidesteps needing qRegisterMetaType for them. + QObject::connect ( &Receiver, &CProtocol::LicenceRequired, [this] ( ELicenceType eLicenceType ) { + strlReceivedLog << QStringLiteral ( "LicenceRequired(%1)" ).arg ( static_cast ( eLicenceType ) ); + } ); + + QObject::connect ( &Receiver, &CProtocol::VersionAndOSReceived, [this] ( COSUtil::EOpSystemType eOSType, QString strVersion ) { + strlReceivedLog << QStringLiteral ( "VersionAndOSReceived(%1, %2)" ).arg ( static_cast ( eOSType ) ).arg ( strVersion ); + } ); + + QObject::connect ( &Receiver, &CProtocol::RecorderStateReceived, [this] ( ERecorderState eRecorderState ) { + strlReceivedLog << QStringLiteral ( "RecorderStateReceived(%1)" ).arg ( static_cast ( eRecorderState ) ); + } ); + } + + QStringList strlSentFrames; + CVector vecbyLastSentFrame; + QStringList strlReceivedLog; + + int iReceiverAcceptedCount = 0; + int iSenderAcceptedCount = 0; +}; diff --git a/src/test/test.pro b/src/test/test.pro new file mode 100644 index 0000000000..2579685b2e --- /dev/null +++ b/src/test/test.pro @@ -0,0 +1,53 @@ +# Unit tests for the Jamulus core (QtTest based) +# +# Build and run (out of tree builds are recommended): +# mkdir build-test && cd build-test +# qmake ../src/test/test.pro +# make && ./jamulus-test +# +# "make check" is supported as well (CONFIG += testcase). + +TARGET = jamulus-test +TEMPLATE = app + +CONFIG += qt \ + thread \ + console \ + testcase + +CONFIG -= app_bundle + +# the sources under test are compiled with the same language level as the +# regular unix build of Jamulus.pro (C++11) +CONFIG += c++11 + +QT += network \ + testlib + +QT -= gui + +# compile the sources under test in a headless server-only configuration so +# that no GUI or sound interface dependencies are pulled in +DEFINES += APP_VERSION=\\\"unittest\\\" \ + SERVER_ONLY \ + HEADLESS \ + NO_JSON_RPC \ + HAVE_STDINT_H \ + QT_NO_DEPRECATED_WARNINGS + +# same as in Jamulus.pro: prevent the windows.h min/max macros from breaking +# std::min/std::max usage in the sources under test +win32 { + DEFINES += NOMINMAX +} + +INCLUDEPATH += .. + +HEADERS += ../global.h \ + ../protocol.h \ + ../util.h \ + protocoltester.h + +SOURCES += ../protocol.cpp \ + ../util.cpp \ + tst_protocol.cpp diff --git a/src/test/tst_protocol.cpp b/src/test/tst_protocol.cpp new file mode 100644 index 0000000000..24e25c7e82 --- /dev/null +++ b/src/test/tst_protocol.cpp @@ -0,0 +1,194 @@ +/******************************************************************************\ + * Copyright (c) 2026 + * + * Author(s): + * The Jamulus Development Team + * + * Licensed under AGPL 3.0 or any later version. See COPYING for details. + * +\******************************************************************************/ + +#include +#include "protocol.h" +#include "protocoltester.h" + +/* Test cases *****************************************************************/ +class CTestProtocol : public QObject +{ + Q_OBJECT + +private slots: + // On-wire compatibility contract: pinned bytes production emits TODAY. + // A mismatch means the wire format changed -- update the literal only as + // a deliberate, reviewed decision. + void GoldenFrameJitBufSize(); + void GoldenFrameChatText(); + + // frame acceptance contract + void AcceptValidFrame(); + void RejectInvalidFrame_data(); + void RejectInvalidFrame(); + void IgnoreAcknWithEmptyBody(); + + void RoundTripChanGain_data(); + void RoundTripChanGain(); +}; + +void CTestProtocol::GoldenFrameJitBufSize() +{ + // Arrange + CProtocolTester Tester; + + // Act + Tester.Sender.CreateJitBufMes ( 5 ); + + // Assert + QString strExpectedFrame; + strExpectedFrame += "00 00 "; // TAG + strExpectedFrame += "0a 00 "; // message ID: PROTMESSID_JITT_BUF_SIZE (10) + strExpectedFrame += "00 "; // sequence counter (fresh instance -> 0) + strExpectedFrame += "02 00 "; // data length (2 bytes) + strExpectedFrame += "05 00 "; // data: jitter buffer size 5 + strExpectedFrame += "5e 06"; // CRC + + QCOMPARE ( Tester.SentFrames(), QStringList{ strExpectedFrame } ); +} + +void CTestProtocol::GoldenFrameChatText() +{ + // Arrange + CProtocolTester Tester; + + // Act + Tester.Sender.CreateChatTextMes ( QStringLiteral ( "Hi" ) ); + + // Assert + QString strExpectedFrame; + strExpectedFrame += "00 00 "; // TAG + strExpectedFrame += "12 00 "; // message ID: PROTMESSID_CHAT_TEXT (18) + strExpectedFrame += "00 "; // sequence counter (fresh instance -> 0) + strExpectedFrame += "04 00 "; // data length (4 bytes) + strExpectedFrame += "02 00 "; // data: UTF-8 string length (2 bytes) + strExpectedFrame += "48 69 "; // data: UTF-8 bytes ("Hi") + strExpectedFrame += "4a 2c"; // CRC + + QCOMPARE ( Tester.SentFrames(), QStringList{ strExpectedFrame } ); +} + +void CTestProtocol::AcceptValidFrame() +{ + // A genuine, valid production frame must reach ParseMessageBody() on the + // receiving side. + CProtocolTester Tester; + Tester.Sender.CreateChatTextMes ( QStringLiteral ( "frame contract test" ) ); + + QCOMPARE ( Tester.ReceivedAndAcceptedMessageCount(), 1 ); +} + +void CTestProtocol::RejectInvalidFrame_data() +{ + QTest::addColumn ( "baFrame" ); + + // a real, production generated frame to mutate below + CProtocolTester Tester; + Tester.Sender.CreateChatTextMes ( QStringLiteral ( "frame contract test" ) ); + + const CVector vecbyValidFrame = Tester.LastSentFrame(); + const QByteArray baValidFrame = CProtocolTester::ToByteArray ( vecbyValidFrame ); + const int iBodyLen = baValidFrame.size() - MESS_LEN_WITHOUT_DATA_BYTE; + + QTest::newRow ( "empty input" ) << QByteArray(); + + QTest::newRow ( "shorter than minimum frame length" ) << baValidFrame.left ( MESS_LEN_WITHOUT_DATA_BYTE - 1 ); + + // one-off bit flip of the first header byte + QByteArray baBadTag = baValidFrame; + baBadTag[0] = static_cast ( baBadTag[0] ^ 0xFF ); + QTest::newRow ( "invalid tag" ) << baBadTag; + + CVector vecbyBadCRC = vecbyValidFrame; + CProtocolTester::CorruptCRC ( vecbyBadCRC ); + QTest::newRow ( "invalid CRC" ) << CProtocolTester::ToByteArray ( vecbyBadCRC ); + + CVector vecbyLenTooLarge = vecbyValidFrame; + CProtocolTester::SetDeclaredLength ( vecbyLenTooLarge, iBodyLen + 1 ); + QTest::newRow ( "declared length larger than data" ) << CProtocolTester::ToByteArray ( vecbyLenTooLarge ); + + CVector vecbyLenTooSmall = vecbyValidFrame; + CProtocolTester::SetDeclaredLength ( vecbyLenTooSmall, iBodyLen - 1 ); + QTest::newRow ( "declared length smaller than data" ) << CProtocolTester::ToByteArray ( vecbyLenTooSmall ); + + CVector vecbyTruncated = vecbyValidFrame; + CProtocolTester::TruncateBy ( vecbyTruncated, 2 ); + QTest::newRow ( "frame truncated on the wire" ) << CProtocolTester::ToByteArray ( vecbyTruncated ); + + // pure junk: no valid frame to mutate, so these two stay raw literals + QTest::newRow ( "junk data" ) << QByteArray ( 50, static_cast ( 0xA5 ) ); + + // oversized junk with a valid tag so that the header decoding is reached + QByteArray baOversized ( MAX_SIZE_BYTES_NETW_BUF, static_cast ( 0xC3 ) ); + baOversized[0] = 0; + baOversized[1] = 0; + QTest::newRow ( "oversized junk data" ) << baOversized; +} + +void CTestProtocol::RejectInvalidFrame() +{ + QFETCH ( QByteArray, baFrame ); + + CProtocolTester Tester; + Tester.SendRawBytes ( baFrame ); + + QCOMPARE ( Tester.ReceivedAndAcceptedMessageCount(), 0 ); +} + +void CTestProtocol::IgnoreAcknWithEmptyBody() +{ + // Regression test for https://github.com/jamulussoftware/jamulus/issues/302 + // (fixed in 024ebb47): an ACKN with a valid checksum but no data caused an + // out-of-bounds read. Still well-formed at the frame level, so it's + // accepted there; ParseMessageBody()'s size check must silently drop it + // -- the ASan/UBSan job is what gives "no OOB" its teeth. + CProtocolTester Seed; + Seed.Sender.CreateChatTextMes ( QStringLiteral ( "frame contract test" ) ); + + CVector vecbyFrame = Seed.LastSentFrame(); + CProtocolTester::ReplaceIdAndBody ( vecbyFrame, PROTMESSID_ACKN, CVector() ); + + CProtocolTester Tester; + Tester.SendRawBytes ( CProtocolTester::ToByteArray ( vecbyFrame ) ); + + QCOMPARE ( Tester.ReceivedAndAcceptedMessageCount(), 1 ); // accepted at the frame level + QVERIFY ( Tester.ReceivedLog().isEmpty() ); // but silently dropped, no signal fired +} + +void CTestProtocol::RoundTripChanGain_data() +{ + QTest::addColumn ( "iChanID" ); + QTest::addColumn ( "fGain" ); + + QTest::newRow ( "zero gain" ) << 0 << 0.0f; + QTest::newRow ( "half gain" ) << 7 << 0.5f; + QTest::newRow ( "full gain" ) << 42 << 1.0f; + QTest::newRow ( "arbitrary gain" ) << 5 << 0.333f; +} + +void CTestProtocol::RoundTripChanGain() +{ + QFETCH ( int, iChanID ); + QFETCH ( float, fGain ); + + // Arrange + CProtocolTester Tester; + + // Act + Tester.Sender.CreateChanGainMes ( iChanID, fGain ); + + // Assert -- also proves no OTHER wired signal fired + QCOMPARE ( Tester.ReceivedLog(), + QStringList{ QStringLiteral ( "ChangeChanGain(%1, %2)" ).arg ( iChanID ).arg ( CProtocolTester::FormatFloat ( fGain ) ) } ); +} + +QTEST_GUILESS_MAIN ( CTestProtocol ) + +#include "tst_protocol.moc" From 7ce4137450d44b6de2c336a1a70925ca8383b639 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:42:17 +0700 Subject: [PATCH 02/18] Add cross-platform unit test runner and summary scripts .github/scripts/run-unit-tests.py is a single Python (stdlib only) driver for building and running the unit test suite across all three CI platforms (Linux, macOS, Windows) with one "build" and one "run" subcommand. On Windows it also bootstraps the MSVC environment itself, using the same env-diffing technique windows/deploy_windows.ps1's Initialize-Build-Environment already uses (call vcvarsall.bat, diff the environment before/after) -- it has to happen again here rather than once in the workflow, since build and run are two separate GitHub Actions steps and an environment set up in one does not survive into the next. "run" is also the test-count gate: after running the binary it parses test-results.xml (stdlib only) and exits nonzero unless the suite reports at least one test and zero failures/errors -- the binary's own exit code is ignored throughout, since it's unreliable on Windows runners, so test-results.xml is the sole source of truth. .github/scripts/summarize-test-results.py parses that same JUnit XML and appends a pass/fail table (plus any failing test names/messages) to the job summary. It is reporting only and always exits 0. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 186 ++++++++++++++++++++++ .github/scripts/summarize-test-results.py | 84 ++++++++++ 2 files changed, 270 insertions(+) create mode 100755 .github/scripts/run-unit-tests.py create mode 100755 .github/scripts/summarize-test-results.py diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py new file mode 100755 index 0000000000..0d8eebd074 --- /dev/null +++ b/.github/scripts/run-unit-tests.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python3 +"""Cross-platform build+run driver for the protocol unit test suite. + +Usage: + python3 run-unit-tests.py build + qmake + make/nmake, out-of-tree in build-test/. On Windows this also + bootstraps the MSVC environment first (see apply_msvc_environment()): + build and run are separate GitHub Actions steps (each a fresh shell), so + that bootstrap has to happen again here rather than once in the workflow. + + python3 run-unit-tests.py run + Runs the built binary, writes test-output.txt and test-results.xml, + prints the text report, then gates on test-results.xml: exits nonzero + unless it reports at least one test and zero failures/errors. The + binary's own exit code is ignored (see cmd_run()). + +Reads from the environment (set by the workflow, from the job's matrix): + QMAKE_BIN -- qmake executable name (default "qmake") + QMAKE_EXTRA_ARGS -- extra qmake command line arguments, shell-quoted as one + string (e.g. QMAKE_CXXFLAGS+="--coverage"); split with + shlex before passing to qmake +""" + +import os +import platform +import re +import shlex +import subprocess +import sys +import xml.etree.ElementTree as ET + +BUILD_DIR = "build-test" +RESULTS_PATH = "test-results.xml" + + +def qmake_extra_args(): + return shlex.split ( os.environ.get ( "QMAKE_EXTRA_ARGS", "" ) ) + + +def run ( cmd, **kwargs ): + print ( "+ " + " ".join ( cmd ) ) + subprocess.run ( cmd, check=True, **kwargs ) + + +def msvc_environment(): + """Returns the environment variables vcvarsall.bat x64 adds/changes, by + diffing `cmd /c set` before and after calling it -- the same env-diffing + technique windows/deploy_windows.ps1's Initialize-Build-Environment uses + for the same purpose.""" + vswhere = r"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" + vs_path = subprocess.run ( + [ + vswhere, + "-latest", + "-prerelease", + "-products", + "*", + "-requires", + "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", + "-property", + "installationPath", + ], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + vcvarsall = os.path.join ( vs_path, "VC", "Auxiliary", "Build", "vcvarsall.bat" ) + + # subprocess.run ( ..., shell=True ) on Windows already runs the string + # through "%COMSPEC% /c " itself, so cmd is passed as raw command + # text here, not prefixed with a literal "cmd /c" -- doing that doubles up + # the cmd.exe nesting and silently swallows vcvarsall.bat's effect on the + # "after" snapshot (before == after, so no variables get applied). + def snapshot ( cmd ): + out = subprocess.run ( cmd, shell=True, capture_output=True, text=True ).stdout + env = {} + for line in out.splitlines(): + m = re.match ( r"^([^=]+)=(.*)$", line ) + if m: + env[m.group ( 1 )] = m.group ( 2 ) + return env + + before = snapshot ( "set" ) + after = snapshot ( 'call "{}" x64 >nul && set'.format ( vcvarsall ) ) + + return { name: value for name, value in after.items() if before.get ( name ) != value } + + +def apply_msvc_environment(): + for name, value in msvc_environment().items(): + os.environ[name] = value + + print ( "VCToolsInstallDir={}".format ( os.environ.get ( "VCToolsInstallDir", "" ) ) ) + + +def cmd_build(): + os.makedirs ( BUILD_DIR, exist_ok=True ) + qmake_bin = os.environ.get ( "QMAKE_BIN", "qmake" ) + + if platform.system() == "Windows": + apply_msvc_environment() + run ( + [ qmake_bin, "..\\src\\test\\test.pro", "CONFIG-=debug_and_release", "CONFIG+=release", "DESTDIR=." ], + cwd=BUILD_DIR, + ) + run ( [ "nmake" ], cwd=BUILD_DIR ) + else: + run ( [ qmake_bin, "../src/test/test.pro" ] + qmake_extra_args(), cwd=BUILD_DIR ) + run ( [ "make", "-j{}".format ( os.cpu_count() or 1 ) ], cwd=BUILD_DIR ) + + +def test_binary_path(): + name = "jamulus-test.exe" if platform.system() == "Windows" else "jamulus-test" + return os.path.join ( BUILD_DIR, name ) + + +def pick_junit_format ( binary ): + # QtTest's junit-flavoured logger is called "xunitxml" on Qt 5.15 and was + # renamed "junitxml" on Qt 6.x -- ask the binary itself via -help instead + # of hardcoding it by Qt version, so this keeps working if the name + # changes again. + help_text = subprocess.run ( [ binary, "-help" ], capture_output=True, text=True ).stdout + return "junitxml" if "junitxml" in help_text else "xunitxml" + + +def read_suites ( path ): + root = ET.parse ( path ).getroot() + return [ root ] if root.tag == "testsuite" else list ( root.findall ( "testsuite" ) ) + + +def gate_on_results(): + if not os.path.exists ( RESULTS_PATH ): + return "gate failed: no {} was produced".format ( RESULTS_PATH ) + + try: + suites = read_suites ( RESULTS_PATH ) + except ET.ParseError as e: + return "gate failed: {} could not be parsed ({})".format ( RESULTS_PATH, e ) + + total_tests = sum ( int ( suite.get ( "tests", 0 ) ) for suite in suites ) + total_failed = sum ( int ( suite.get ( "failures", 0 ) ) + int ( suite.get ( "errors", 0 ) ) for suite in suites ) + + if total_tests == 0: + return "gate failed: {} reported 0 tests".format ( RESULTS_PATH ) + + if total_failed != 0: + return "gate failed: {} of {} tests failed".format ( total_failed, total_tests ) + + print ( "gate passed: {} tests, 0 failures".format ( total_tests ) ) + return None + + +def cmd_run(): + binary = test_binary_path() + junit_format = pick_junit_format ( binary ) + print ( "Using QtTest JUnit logger format: " + junit_format ) + + # "-o file,txt" not "-o -,txt": on windows-latest runners this binary's + # stdout comes back 0 bytes regardless of capture method (suspected: Qt's + # console detection on the inherited handle); QtTest's own file writer + # works there. Using it on Unix too avoids a platform branch. + subprocess.run ( [ binary, "-o", "test-output.txt,txt", "-o", "test-results.xml," + junit_format ] ) + + if os.path.exists ( "test-output.txt" ): + with open ( "test-output.txt", encoding="utf-8", errors="replace" ) as f: + sys.stdout.write ( f.read() ) + + # The binary's own exit code is unreliable on Windows runners, so it's + # ignored on every platform; test-results.xml is the sole source of truth. + error = gate_on_results() + if error: + sys.exit ( error ) + + +def main(): + if len ( sys.argv ) != 2 or sys.argv[1] not in ( "build", "run" ): + sys.exit ( "usage: run-unit-tests.py " ) + + if sys.argv[1] == "build": + cmd_build() + else: + cmd_run() + + +if __name__ == "__main__": + main() diff --git a/.github/scripts/summarize-test-results.py b/.github/scripts/summarize-test-results.py new file mode 100755 index 0000000000..4146b61a30 --- /dev/null +++ b/.github/scripts/summarize-test-results.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +"""Renders test-results.xml (the junit-flavoured QtTest report +run-unit-tests.py's "run" subcommand writes) as a per-suite pass/fail table, +plus any failing test names/messages, appended to $GITHUB_STEP_SUMMARY. +Reporting only: always exits 0, so a missing or unparsable results file just +shows up in the summary instead of failing this step. +""" + +import os +import sys +import xml.etree.ElementTree as ET + +RESULTS_PATH = "test-results.xml" + + +def read_suites ( path ): + root = ET.parse ( path ).getroot() + return [ root ] if root.tag == "testsuite" else list ( root.findall ( "testsuite" ) ) + + +def write_summary ( summary_path, lines ): + if not summary_path: + sys.stdout.writelines ( lines ) + return + + with open ( summary_path, "a", encoding="utf-8" ) as f: + f.writelines ( lines ) + + +def main(): + job_name = os.environ.get ( "JOB_NAME", "unit tests" ) + summary_path = os.environ.get ( "GITHUB_STEP_SUMMARY" ) + + lines = [ "### JUnit results: {}\n\n".format ( job_name ) ] + + suites = None + if not os.path.exists ( RESULTS_PATH ): + lines.append ( "_No {} was produced (the build or test run likely failed before it could be written)._\n\n".format ( RESULTS_PATH ) ) + else: + try: + suites = read_suites ( RESULTS_PATH ) + except ET.ParseError as e: + lines.append ( "_{} could not be parsed ({})._\n\n".format ( RESULTS_PATH, e ) ) + + if suites is None: + write_summary ( summary_path, lines ) + return + + lines.append ( "| Suite | Tests | Passed | Failed | Skipped | Time (s) |\n" ) + lines.append ( "|---|---|---|---|---|---|\n" ) + + failures = [] + + for suite in suites: + tests = int ( suite.get ( "tests", 0 ) ) + failed = int ( suite.get ( "failures", 0 ) ) + int ( suite.get ( "errors", 0 ) ) + skipped = int ( suite.get ( "skipped", 0 ) ) + passed = tests - failed - skipped + status = "PASS" if failed == 0 else "FAIL" + + lines.append ( + "| {} {} | {} | {} | {} | {} | {} |\n".format ( status, suite.get ( "name" ), tests, passed, failed, skipped, suite.get ( "time", "?" ) ) + ) + + for testcase in suite.findall ( "testcase" ): + node = testcase.find ( "failure" ) + if node is None: + node = testcase.find ( "error" ) + if node is not None: + failures.append ( ( testcase.get ( "name" ), ( node.get ( "message" ) or "" ).strip() ) ) + + if failures: + lines.append ( "\n
Failed tests\n\n" ) + for name, message in failures: + lines.append ( "- `{}`: {}\n".format ( name, message ) ) + lines.append ( "\n
\n" ) + + lines.append ( "\n" ) + + write_summary ( summary_path, lines ) + + +if __name__ == "__main__": + main() From 58bd238fdfac5e2aeea73a3a0fbfae6bc4e986c8 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:42:27 +0700 Subject: [PATCH 03/18] Add unit test CI workflow .github/workflows/unit-tests.yml runs the new suite on every push and pull request touching src/**, this workflow file, or the scripts it calls (plus workflow_dispatch for manual runs), mirroring the trigger shape of the repository's other main-branch check workflows. A 6-job matrix covers: - Linux (Qt 5.15, ubuntu-22.04) -- proves the suite's Qt >= 5.12 floor - Linux (Qt 6, ubuntu-24.04) - macOS (Qt 6) and Windows (Qt 6, MSVC) -- Qt is set up by reusing autobuild.yml's own setup scripts (.github/autobuild/mac.sh, windows.ps1) and its exact cache key/paths, so this job shares that cache instead of keeping a separate one; heavier than this suite strictly needs (extra Qt modules, a 32-bit Qt, jom on Windows) in exchange for that shared cache - Linux (Qt 6, ASan/UBSan) -- same toolchain with sanitizers enabled via qmake command line flags, to catch memory/UB issues a plain build wouldn't - Linux (Qt 6, coverage) -- same toolchain instrumented with gcov, producing an HTML report artifact plus a Markdown summary table for src/ (excluding the test suite itself) Every job builds and runs the suite via run-unit-tests.py, which is also the gate; Summarize test results (always run) and the coverage steps (always run, coverage jobs only) still render their output even if that gate failed. The workflow only needs `permissions: contents: read` throughout. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/workflows/unit-tests.yml | 221 +++++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 .github/workflows/unit-tests.yml diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml new file mode 100644 index 0000000000..d007a955ea --- /dev/null +++ b/.github/workflows/unit-tests.yml @@ -0,0 +1,221 @@ +name: Unit Tests + +on: + workflow_dispatch: + push: + branches: [ main ] + paths: + - 'src/**' + - '.github/workflows/unit-tests.yml' + - '.github/scripts/**' + pull_request: + branches: [ main ] + paths: + - 'src/**' + - '.github/workflows/unit-tests.yml' + - '.github/scripts/**' + +permissions: + contents: read + +env: + # Must match autobuild.yml's macOS/Windows QT_VERSION -- Qt setup below + # reuses those jobs' scripts and cache. + QT_VERSION: 6.10.2 + + # gcovr version used by the coverage job below, pinned for reproducible output. + GCOVR_VERSION: 8.6 + +jobs: + unit-tests: + name: ${{ matrix.config.name }} + runs-on: ${{ matrix.config.runs_on }} + strategy: + fail-fast: false + matrix: + config: + # `id` is a short, stable slug (no spaces/parens, unlike `name`) + # used for per-job artifact names (junit-, see the "Upload + # JUnit XML" step below) so parallel matrix jobs never collide. + # + # Oldest supported Qt line: distro Qt 5.15 proves the Qt >= 5.12 + # compatibility floor of the test suite. + - name: Linux (Qt 5, ubuntu-22.04) + id: linux-qt5 + runs_on: ubuntu-22.04 + qt_packages: qtbase5-dev qt5-qmake + qmake: qmake + - name: Linux (Qt 6, ubuntu-24.04) + id: linux-qt6 + runs_on: ubuntu-24.04 + qt_packages: qt6-base-dev + qmake: qmake6 + # macos-15 with Xcode 16.3.0 mirrors the autobuild workflow's macOS + # configuration; the Xcode 26 toolchain of newer images cannot + # compile the Qt 6 headers yet (implicit __yield declaration error). + - name: macOS (Qt 6) + id: macos-qt6 + runs_on: macos-15 + xcode_version: 16.3.0 + qmake: qmake + # windows-2025 matches autobuild.yml's Windows job -- same OS image + # as the Qt/toolchain cache this job shares with it. + - name: Windows (Qt 6, MSVC) + id: windows-qt6 + runs_on: windows-2025 + qmake: qmake + + # Same Qt6/ubuntu-24.04 toolchain as the Linux Qt6 job above, plus + # GCC's sanitizers -- appended via qmake_extra_args rather than + # edited into test.pro, so no other job is affected. + - name: Linux (Qt 6, ASan/UBSan) + id: linux-qt6-asan + runs_on: ubuntu-24.04 + qt_packages: qt6-base-dev + qmake: qmake6 + qmake_extra_args: >- + QMAKE_CXXFLAGS+="-fsanitize=address,undefined" + QMAKE_LFLAGS+="-fsanitize=address,undefined" + + # Same Qt6/ubuntu-24.04 toolchain, built with GCC's --coverage flag + # for the line/function/branch report generated below. + - name: Linux (Qt 6, coverage) + id: linux-qt6-coverage + runs_on: ubuntu-24.04 + qt_packages: qt6-base-dev + qmake: qmake6 + coverage: true + qmake_extra_args: >- + QMAKE_CXXFLAGS+="--coverage" + QMAKE_LFLAGS+="--coverage" + + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Install Qt (Linux distro packages) + if: runner.os == 'Linux' + run: | + sudo apt-get -qq update + sudo apt-get -qq --no-install-recommends -y install ${{ matrix.config.qt_packages }} + + - name: Select Xcode version + if: runner.os == 'macOS' + uses: maxim-lobanov/setup-xcode@ed7a3b1fda3918c0306d1b724322adc0b8cc0a90 # v1.6.0 + with: + xcode-version: ${{ matrix.config.xcode_version }} + + # Same cache path/key as autobuild.yml's "MacOS (artifacts)" job, so + # this job shares that cache instead of keeping a separate one. + - name: Cache Qt (macOS) + if: runner.os == 'macOS' + uses: actions/cache@v6 + with: + path: | + ~/qt + ~/Library/Cache/jamulus-dependencies + key: macos-${{ hashFiles('.github/workflows/autobuild.yml', '.github/autobuild/mac.sh', 'mac/deploy_mac.sh') }}-QT_VERSION=6.10.2 SIGN_IF_POSSIBLE=1 TARGET_ARCHS="x86_64 arm64" ./.github/autobuild/mac.sh + + # Same cache path/key as autobuild.yml's "Windows (artifact+codeQL)" + # job, so this job shares that cache instead of keeping a separate one. + - name: Cache Qt (Windows) + if: runner.os == 'Windows' + uses: actions/cache@v6 + with: + path: | + C:\Qt + C:\ChocoCache + C:\AutobuildCache + ${{ github.workspace }}\libs\NSIS\NSIS-source + ${{ github.workspace }}\libs\ASIOSDK2 + key: windows-${{ hashFiles('.github/workflows/autobuild.yml', '.github/autobuild/windows.ps1', 'windows/deploy_windows.ps1') }}-powershell .\.github\autobuild\windows.ps1 -Stage + + # Reuses autobuild's own Qt setup instead of a separate aqtinstall call, + # so the cache above is genuinely shared rather than duplicated -- + # heavier than this suite needs (qttools/qttranslations/qtmultimedia + # besides qtbase). + - name: Install Qt (macOS, via autobuild's mac.sh) + if: runner.os == 'macOS' + env: + JAMULUS_BUILD_VERSION: 0.0.0 # required by the script's validation; unused by "setup" + run: ./.github/autobuild/mac.sh setup + + - name: Add Qt to PATH (macOS) + if: runner.os == 'macOS' + run: | + # macos/ is the Qt6 layout, clang_64/ is Qt5's; only one will exist. + echo "$HOME/qt/${{ env.QT_VERSION }}/macos/bin" >> "$GITHUB_PATH" + echo "$HOME/qt/${{ env.QT_VERSION }}/clang_64/bin" >> "$GITHUB_PATH" + + # Reuses autobuild's own Qt setup instead of a separate aqtinstall call, + # so the cache above is genuinely shared rather than duplicated -- + # heavier than this suite needs (a 32-bit Qt, jom, full module set + # besides qtbase). + - name: Install Qt (Windows, via autobuild's windows.ps1) + if: runner.os == 'Windows' + shell: pwsh + env: + JAMULUS_BUILD_VERSION: 0.0.0 # required by the script's validation; unused by "setup" + run: powershell .\.github\autobuild\windows.ps1 -Stage setup + + - name: Add Qt to PATH (Windows) + if: runner.os == 'Windows' + shell: pwsh + run: | + # windows.ps1 hardcodes this Qt/MSVC pairing internally (no env + # override), so it must be kept in sync with it by hand. + Add-Content -Path $env:GITHUB_PATH -Value "C:\Qt\${{ env.QT_VERSION }}\msvc2022_64\bin" + + - name: Build unit tests + env: + QMAKE_BIN: ${{ matrix.config.qmake }} + QMAKE_EXTRA_ARGS: ${{ matrix.config.qmake_extra_args }} + run: python3 .github/scripts/run-unit-tests.py build + + # crash the test process when ASan/UBSan complains (no-op when disabled) + - name: Run unit tests + env: + ASAN_OPTIONS: halt_on_error=1 + UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1 + run: python3 .github/scripts/run-unit-tests.py run + + # Coverage for src/ only (excluding tests). `always()` so this still + # runs even if Run unit tests failed on a real test failure, not a crash. + - name: Generate coverage report + if: always() && matrix.config.coverage + run: | + mkdir coverage-html + pipx run "gcovr==${{ env.GCOVR_VERSION }}" \ + --root "$GITHUB_WORKSPACE" \ + --filter "$GITHUB_WORKSPACE/src/" \ + --exclude "$GITHUB_WORKSPACE/src/test/" \ + --object-directory build-test \ + --html-details coverage-html/index.html \ + --markdown coverage-summary.md \ + --print-summary + cat coverage-summary.md >> "$GITHUB_STEP_SUMMARY" + + - name: Upload coverage report + if: always() && matrix.config.coverage + uses: actions/upload-artifact@v7 + with: + name: coverage-html + path: coverage-html + retention-days: 31 + if-no-files-found: error + + # `if: always()` so this still runs even if Run unit tests failed. + - name: Summarize test results + if: always() + env: + JOB_NAME: ${{ matrix.config.name }} + run: python3 .github/scripts/summarize-test-results.py + + - name: Upload JUnit XML + if: always() + uses: actions/upload-artifact@v7 + with: + name: junit-${{ matrix.config.id }} + path: test-results.xml + retention-days: 31 + if-no-files-found: warn From 15212256a1b394ca7538ac8f6f85b7b6874c456f Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 22:11:59 +0700 Subject: [PATCH 04/18] Address review feedback: clamp TruncateBy, fail fast on vcvarsall errors Both from PR review on jamulussoftware/jamulus#3828: - src/test/protocoltester.h: CProtocolTester::TruncateBy() now clamps the resize at 0, so truncating by more than the frame's size yields an empty frame instead of a negative resize (which wraps to a huge size_t and would abort/corrupt memory). - .github/scripts/run-unit-tests.py: msvc_environment()'s snapshot() helper now checks the subprocess return code and fails fast with the command plus its captured stdout/stderr if it's nonzero, instead of letting a broken vcvarsall.bat call surface later as a confusing compiler-not-found error. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 7 +++++-- src/test/protocoltester.h | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py index 0d8eebd074..21d79a32b3 100755 --- a/.github/scripts/run-unit-tests.py +++ b/.github/scripts/run-unit-tests.py @@ -72,9 +72,12 @@ def msvc_environment(): # the cmd.exe nesting and silently swallows vcvarsall.bat's effect on the # "after" snapshot (before == after, so no variables get applied). def snapshot ( cmd ): - out = subprocess.run ( cmd, shell=True, capture_output=True, text=True ).stdout + result = subprocess.run ( cmd, shell=True, capture_output=True, text=True ) + if result.returncode != 0: + sys.exit ( "command failed (exit {}): {}\nstdout:\n{}\nstderr:\n{}".format ( result.returncode, cmd, result.stdout, result.stderr ) ) + env = {} - for line in out.splitlines(): + for line in result.stdout.splitlines(): m = re.match ( r"^([^=]+)=(.*)$", line ) if m: env[m.group ( 1 )] = m.group ( 2 ) diff --git a/src/test/protocoltester.h b/src/test/protocoltester.h index f73ca408f4..7315efd576 100644 --- a/src/test/protocoltester.h +++ b/src/test/protocoltester.h @@ -94,8 +94,9 @@ class CProtocolTester */ // chops iBytes off the end of vecbyFrame, e.g. to cut into the CRC or the - // body - static void TruncateBy ( CVector& vecbyFrame, const int iBytes ) { vecbyFrame.resize ( vecbyFrame.Size() - iBytes ); } + // body; clamped at 0 so an oversize iBytes yields an empty frame instead + // of a negative resize (which wraps to a huge size_t). + static void TruncateBy ( CVector& vecbyFrame, const int iBytes ) { vecbyFrame.resize ( std::max ( vecbyFrame.Size() - iBytes, 0 ) ); } // flips every bit of the trailing CRC byte so the checksum no longer // matches the header plus body From 94cbe34db204a5bdb8c70c2b210eb8696177d8e2 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 22:34:35 +0700 Subject: [PATCH 05/18] Include directly in protocoltester.h The header uses std::max but relied on transitive inclusion via protocol.h -> util.h. Include it directly so the header stays self-contained (same practice as util.h), per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- src/test/protocoltester.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/protocoltester.h b/src/test/protocoltester.h index 7315efd576..a368277a64 100644 --- a/src/test/protocoltester.h +++ b/src/test/protocoltester.h @@ -11,6 +11,7 @@ #pragma once #include +#include #include "protocol.h" /* CProtocolTester **************************************************************/ From 8e2853b19756768409e344318682497a7848f701 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 22:52:31 +0700 Subject: [PATCH 06/18] Build the macOS Qt cache key from env.QT_VERSION The key hardcoded the version string alongside the env.QT_VERSION definition above, so the two could drift after a Qt update. The interpolated key is byte-identical today, keeping the cache shared with autobuild.yml's macOS job. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/workflows/unit-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index d007a955ea..f42a3775b0 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -114,7 +114,7 @@ jobs: path: | ~/qt ~/Library/Cache/jamulus-dependencies - key: macos-${{ hashFiles('.github/workflows/autobuild.yml', '.github/autobuild/mac.sh', 'mac/deploy_mac.sh') }}-QT_VERSION=6.10.2 SIGN_IF_POSSIBLE=1 TARGET_ARCHS="x86_64 arm64" ./.github/autobuild/mac.sh + key: macos-${{ hashFiles('.github/workflows/autobuild.yml', '.github/autobuild/mac.sh', 'mac/deploy_mac.sh') }}-QT_VERSION=${{ env.QT_VERSION }} SIGN_IF_POSSIBLE=1 TARGET_ARCHS="x86_64 arm64" ./.github/autobuild/mac.sh # Same cache path/key as autobuild.yml's "Windows (artifact+codeQL)" # job, so this job shares that cache instead of keeping a separate one. From ccbb7f5605cbd16cfc23d41685ee69f504a718c4 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:23:10 +0700 Subject: [PATCH 07/18] Address review feedback: credit author, surface deprecation warnings Set the Author(s) header of the new test files to dtinth, and drop QT_NO_DEPRECATED_WARNINGS from the test build: test code should surface problems, not hide them. This makes one existing deprecation warning visible (util.cpp uses QLibraryInfo::location(), deprecated in Qt 6) -- a production cleanup to propose separately. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- src/test/protocoltester.h | 2 +- src/test/test.pro | 3 +-- src/test/tst_protocol.cpp | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/protocoltester.h b/src/test/protocoltester.h index a368277a64..d400741588 100644 --- a/src/test/protocoltester.h +++ b/src/test/protocoltester.h @@ -2,7 +2,7 @@ * Copyright (c) 2026 * * Author(s): - * The Jamulus Development Team + * dtinth * * Licensed under AGPL 3.0 or any later version. See COPYING for details. * diff --git a/src/test/test.pro b/src/test/test.pro index 2579685b2e..2507c532e5 100644 --- a/src/test/test.pro +++ b/src/test/test.pro @@ -32,8 +32,7 @@ DEFINES += APP_VERSION=\\\"unittest\\\" \ SERVER_ONLY \ HEADLESS \ NO_JSON_RPC \ - HAVE_STDINT_H \ - QT_NO_DEPRECATED_WARNINGS + HAVE_STDINT_H # same as in Jamulus.pro: prevent the windows.h min/max macros from breaking # std::min/std::max usage in the sources under test diff --git a/src/test/tst_protocol.cpp b/src/test/tst_protocol.cpp index 24e25c7e82..bef80336d1 100644 --- a/src/test/tst_protocol.cpp +++ b/src/test/tst_protocol.cpp @@ -2,7 +2,7 @@ * Copyright (c) 2026 * * Author(s): - * The Jamulus Development Team + * dtinth * * Licensed under AGPL 3.0 or any later version. See COPYING for details. * From 0b5c9dcb7439aaed94223bb36277dd37e8a43322 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:29:58 +0700 Subject: [PATCH 08/18] Delete stale test reports before running the suite A binary that crashed before writing output could otherwise let the JUnit gate pass on a leftover test-results.xml from a previous local run. Remove both report files up front and refer to them through the existing constants throughout. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py index 21d79a32b3..c5bc292c45 100755 --- a/.github/scripts/run-unit-tests.py +++ b/.github/scripts/run-unit-tests.py @@ -31,6 +31,7 @@ BUILD_DIR = "build-test" RESULTS_PATH = "test-results.xml" +OUTPUT_PATH = "test-output.txt" def qmake_extra_args(): @@ -158,14 +159,20 @@ def cmd_run(): junit_format = pick_junit_format ( binary ) print ( "Using QtTest JUnit logger format: " + junit_format ) + # remove stale reports so the gate below can only ever see this run's + # output, even if the binary crashes before writing anything + for path in [ RESULTS_PATH, OUTPUT_PATH ]: + if os.path.exists ( path ): + os.remove ( path ) + # "-o file,txt" not "-o -,txt": on windows-latest runners this binary's # stdout comes back 0 bytes regardless of capture method (suspected: Qt's # console detection on the inherited handle); QtTest's own file writer # works there. Using it on Unix too avoids a platform branch. - subprocess.run ( [ binary, "-o", "test-output.txt,txt", "-o", "test-results.xml," + junit_format ] ) + subprocess.run ( [ binary, "-o", OUTPUT_PATH + ",txt", "-o", RESULTS_PATH + "," + junit_format ] ) - if os.path.exists ( "test-output.txt" ): - with open ( "test-output.txt", encoding="utf-8", errors="replace" ) as f: + if os.path.exists ( OUTPUT_PATH ): + with open ( OUTPUT_PATH, encoding="utf-8", errors="replace" ) as f: sys.stdout.write ( f.read() ) # The binary's own exit code is unreliable on Windows runners, so it's From 6094fbe36921a528288bbd2778a386a2515b50de Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:39:36 +0700 Subject: [PATCH 09/18] Use the project's full AGPL license header in the test files These are the first source files created after the AGPL transition (eb172d47), so they carry the standard boilerplate minus the paragraphs that only describe pre-transition GPL code, which these files do not contain. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- src/test/protocoltester.h | 18 +++++++++++++++++- src/test/tst_protocol.cpp | 18 +++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/test/protocoltester.h b/src/test/protocoltester.h index d400741588..f61a706225 100644 --- a/src/test/protocoltester.h +++ b/src/test/protocoltester.h @@ -4,7 +4,23 @@ * Author(s): * dtinth * - * Licensed under AGPL 3.0 or any later version. See COPYING for details. + * As of Jamulus 3.12.1dev (commit eb172d47): All new source code contributions must be licensed + * under AGPL 3.0 or any later version. + * + ****************************************************************************** + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . * \******************************************************************************/ diff --git a/src/test/tst_protocol.cpp b/src/test/tst_protocol.cpp index bef80336d1..ea815b99ed 100644 --- a/src/test/tst_protocol.cpp +++ b/src/test/tst_protocol.cpp @@ -4,7 +4,23 @@ * Author(s): * dtinth * - * Licensed under AGPL 3.0 or any later version. See COPYING for details. + * As of Jamulus 3.12.1dev (commit eb172d47): All new source code contributions must be licensed + * under AGPL 3.0 or any later version. + * + ****************************************************************************** + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . * \******************************************************************************/ From e859ee98668f1d57037cb5e7bea3829f862ae56b Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 00:00:25 +0700 Subject: [PATCH 10/18] Pass QMAKE_EXTRA_ARGS to qmake on Windows too The Unix branch already appends qmake_extra_args(); the Windows branch ignored it, which was harmless (only the Linux ASan job sets the variable) but inconsistent, and would silently drop the args from any future Windows build variant. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py index c5bc292c45..3830f98895 100755 --- a/.github/scripts/run-unit-tests.py +++ b/.github/scripts/run-unit-tests.py @@ -104,7 +104,7 @@ def cmd_build(): if platform.system() == "Windows": apply_msvc_environment() run ( - [ qmake_bin, "..\\src\\test\\test.pro", "CONFIG-=debug_and_release", "CONFIG+=release", "DESTDIR=." ], + [ qmake_bin, "..\\src\\test\\test.pro", "CONFIG-=debug_and_release", "CONFIG+=release", "DESTDIR=." ] + qmake_extra_args(), cwd=BUILD_DIR, ) run ( [ "nmake" ], cwd=BUILD_DIR ) From 9a26abc7a4a443107e8915136cde4e62f84ee88f Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 00:14:51 +0700 Subject: [PATCH 11/18] Name the results file in the failed-tests gate message The other two gate messages already include it. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py index 3830f98895..564fc9bc37 100755 --- a/.github/scripts/run-unit-tests.py +++ b/.github/scripts/run-unit-tests.py @@ -148,7 +148,7 @@ def gate_on_results(): return "gate failed: {} reported 0 tests".format ( RESULTS_PATH ) if total_failed != 0: - return "gate failed: {} of {} tests failed".format ( total_failed, total_tests ) + return "gate failed: {} reported {} of {} tests failed".format ( RESULTS_PATH, total_failed, total_tests ) print ( "gate passed: {} tests, 0 failures".format ( total_tests ) ) return None From eb2145b7d3c4c2d353fbc759bb1d889a8e190615 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 00:24:14 +0700 Subject: [PATCH 12/18] Format the CI scripts with standard Python style The scripts had inherited the C++ sources' space-inside-parens style; the repo's existing Python (.github/autobuild/get_build_vars.py, tools/generate_json_rpc_docs.py) uses conventional PEP8 formatting, so follow that. Reformatted with black; no behavior change (AST-identical before and after). Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 115 +++++++++++++--------- .github/scripts/summarize-test-results.py | 76 ++++++++------ 2 files changed, 111 insertions(+), 80 deletions(-) diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py index 564fc9bc37..1235286061 100755 --- a/.github/scripts/run-unit-tests.py +++ b/.github/scripts/run-unit-tests.py @@ -35,12 +35,12 @@ def qmake_extra_args(): - return shlex.split ( os.environ.get ( "QMAKE_EXTRA_ARGS", "" ) ) + return shlex.split(os.environ.get("QMAKE_EXTRA_ARGS", "")) -def run ( cmd, **kwargs ): - print ( "+ " + " ".join ( cmd ) ) - subprocess.run ( cmd, check=True, **kwargs ) +def run(cmd, **kwargs): + print("+ " + " ".join(cmd)) + subprocess.run(cmd, check=True, **kwargs) def msvc_environment(): @@ -49,7 +49,7 @@ def msvc_environment(): technique windows/deploy_windows.ps1's Initialize-Build-Environment uses for the same purpose.""" vswhere = r"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" - vs_path = subprocess.run ( + vs_path = subprocess.run( [ vswhere, "-latest", @@ -65,126 +65,143 @@ def msvc_environment(): capture_output=True, text=True, ).stdout.strip() - vcvarsall = os.path.join ( vs_path, "VC", "Auxiliary", "Build", "vcvarsall.bat" ) + vcvarsall = os.path.join(vs_path, "VC", "Auxiliary", "Build", "vcvarsall.bat") # subprocess.run ( ..., shell=True ) on Windows already runs the string # through "%COMSPEC% /c " itself, so cmd is passed as raw command # text here, not prefixed with a literal "cmd /c" -- doing that doubles up # the cmd.exe nesting and silently swallows vcvarsall.bat's effect on the # "after" snapshot (before == after, so no variables get applied). - def snapshot ( cmd ): - result = subprocess.run ( cmd, shell=True, capture_output=True, text=True ) + def snapshot(cmd): + result = subprocess.run(cmd, shell=True, capture_output=True, text=True) if result.returncode != 0: - sys.exit ( "command failed (exit {}): {}\nstdout:\n{}\nstderr:\n{}".format ( result.returncode, cmd, result.stdout, result.stderr ) ) + sys.exit( + "command failed (exit {}): {}\nstdout:\n{}\nstderr:\n{}".format( + result.returncode, cmd, result.stdout, result.stderr + ) + ) env = {} for line in result.stdout.splitlines(): - m = re.match ( r"^([^=]+)=(.*)$", line ) + m = re.match(r"^([^=]+)=(.*)$", line) if m: - env[m.group ( 1 )] = m.group ( 2 ) + env[m.group(1)] = m.group(2) return env - before = snapshot ( "set" ) - after = snapshot ( 'call "{}" x64 >nul && set'.format ( vcvarsall ) ) + before = snapshot("set") + after = snapshot('call "{}" x64 >nul && set'.format(vcvarsall)) - return { name: value for name, value in after.items() if before.get ( name ) != value } + return {name: value for name, value in after.items() if before.get(name) != value} def apply_msvc_environment(): for name, value in msvc_environment().items(): os.environ[name] = value - print ( "VCToolsInstallDir={}".format ( os.environ.get ( "VCToolsInstallDir", "" ) ) ) + print("VCToolsInstallDir={}".format(os.environ.get("VCToolsInstallDir", ""))) def cmd_build(): - os.makedirs ( BUILD_DIR, exist_ok=True ) - qmake_bin = os.environ.get ( "QMAKE_BIN", "qmake" ) + os.makedirs(BUILD_DIR, exist_ok=True) + qmake_bin = os.environ.get("QMAKE_BIN", "qmake") if platform.system() == "Windows": apply_msvc_environment() - run ( - [ qmake_bin, "..\\src\\test\\test.pro", "CONFIG-=debug_and_release", "CONFIG+=release", "DESTDIR=." ] + qmake_extra_args(), + run( + [ + qmake_bin, + "..\\src\\test\\test.pro", + "CONFIG-=debug_and_release", + "CONFIG+=release", + "DESTDIR=.", + ] + + qmake_extra_args(), cwd=BUILD_DIR, ) - run ( [ "nmake" ], cwd=BUILD_DIR ) + run(["nmake"], cwd=BUILD_DIR) else: - run ( [ qmake_bin, "../src/test/test.pro" ] + qmake_extra_args(), cwd=BUILD_DIR ) - run ( [ "make", "-j{}".format ( os.cpu_count() or 1 ) ], cwd=BUILD_DIR ) + run([qmake_bin, "../src/test/test.pro"] + qmake_extra_args(), cwd=BUILD_DIR) + run(["make", "-j{}".format(os.cpu_count() or 1)], cwd=BUILD_DIR) def test_binary_path(): name = "jamulus-test.exe" if platform.system() == "Windows" else "jamulus-test" - return os.path.join ( BUILD_DIR, name ) + return os.path.join(BUILD_DIR, name) -def pick_junit_format ( binary ): +def pick_junit_format(binary): # QtTest's junit-flavoured logger is called "xunitxml" on Qt 5.15 and was # renamed "junitxml" on Qt 6.x -- ask the binary itself via -help instead # of hardcoding it by Qt version, so this keeps working if the name # changes again. - help_text = subprocess.run ( [ binary, "-help" ], capture_output=True, text=True ).stdout + help_text = subprocess.run([binary, "-help"], capture_output=True, text=True).stdout return "junitxml" if "junitxml" in help_text else "xunitxml" -def read_suites ( path ): - root = ET.parse ( path ).getroot() - return [ root ] if root.tag == "testsuite" else list ( root.findall ( "testsuite" ) ) +def read_suites(path): + root = ET.parse(path).getroot() + return [root] if root.tag == "testsuite" else list(root.findall("testsuite")) def gate_on_results(): - if not os.path.exists ( RESULTS_PATH ): - return "gate failed: no {} was produced".format ( RESULTS_PATH ) + if not os.path.exists(RESULTS_PATH): + return "gate failed: no {} was produced".format(RESULTS_PATH) try: - suites = read_suites ( RESULTS_PATH ) + suites = read_suites(RESULTS_PATH) except ET.ParseError as e: - return "gate failed: {} could not be parsed ({})".format ( RESULTS_PATH, e ) + return "gate failed: {} could not be parsed ({})".format(RESULTS_PATH, e) - total_tests = sum ( int ( suite.get ( "tests", 0 ) ) for suite in suites ) - total_failed = sum ( int ( suite.get ( "failures", 0 ) ) + int ( suite.get ( "errors", 0 ) ) for suite in suites ) + total_tests = sum(int(suite.get("tests", 0)) for suite in suites) + total_failed = sum( + int(suite.get("failures", 0)) + int(suite.get("errors", 0)) for suite in suites + ) if total_tests == 0: - return "gate failed: {} reported 0 tests".format ( RESULTS_PATH ) + return "gate failed: {} reported 0 tests".format(RESULTS_PATH) if total_failed != 0: - return "gate failed: {} reported {} of {} tests failed".format ( RESULTS_PATH, total_failed, total_tests ) + return "gate failed: {} reported {} of {} tests failed".format( + RESULTS_PATH, total_failed, total_tests + ) - print ( "gate passed: {} tests, 0 failures".format ( total_tests ) ) + print("gate passed: {} tests, 0 failures".format(total_tests)) return None def cmd_run(): binary = test_binary_path() - junit_format = pick_junit_format ( binary ) - print ( "Using QtTest JUnit logger format: " + junit_format ) + junit_format = pick_junit_format(binary) + print("Using QtTest JUnit logger format: " + junit_format) # remove stale reports so the gate below can only ever see this run's # output, even if the binary crashes before writing anything - for path in [ RESULTS_PATH, OUTPUT_PATH ]: - if os.path.exists ( path ): - os.remove ( path ) + for path in [RESULTS_PATH, OUTPUT_PATH]: + if os.path.exists(path): + os.remove(path) # "-o file,txt" not "-o -,txt": on windows-latest runners this binary's # stdout comes back 0 bytes regardless of capture method (suspected: Qt's # console detection on the inherited handle); QtTest's own file writer # works there. Using it on Unix too avoids a platform branch. - subprocess.run ( [ binary, "-o", OUTPUT_PATH + ",txt", "-o", RESULTS_PATH + "," + junit_format ] ) + subprocess.run( + [binary, "-o", OUTPUT_PATH + ",txt", "-o", RESULTS_PATH + "," + junit_format] + ) - if os.path.exists ( OUTPUT_PATH ): - with open ( OUTPUT_PATH, encoding="utf-8", errors="replace" ) as f: - sys.stdout.write ( f.read() ) + if os.path.exists(OUTPUT_PATH): + with open(OUTPUT_PATH, encoding="utf-8", errors="replace") as f: + sys.stdout.write(f.read()) # The binary's own exit code is unreliable on Windows runners, so it's # ignored on every platform; test-results.xml is the sole source of truth. error = gate_on_results() if error: - sys.exit ( error ) + sys.exit(error) def main(): - if len ( sys.argv ) != 2 or sys.argv[1] not in ( "build", "run" ): - sys.exit ( "usage: run-unit-tests.py " ) + if len(sys.argv) != 2 or sys.argv[1] not in ("build", "run"): + sys.exit("usage: run-unit-tests.py ") if sys.argv[1] == "build": cmd_build() diff --git a/.github/scripts/summarize-test-results.py b/.github/scripts/summarize-test-results.py index 4146b61a30..3c9e7e4db0 100755 --- a/.github/scripts/summarize-test-results.py +++ b/.github/scripts/summarize-test-results.py @@ -13,71 +13,85 @@ RESULTS_PATH = "test-results.xml" -def read_suites ( path ): - root = ET.parse ( path ).getroot() - return [ root ] if root.tag == "testsuite" else list ( root.findall ( "testsuite" ) ) +def read_suites(path): + root = ET.parse(path).getroot() + return [root] if root.tag == "testsuite" else list(root.findall("testsuite")) -def write_summary ( summary_path, lines ): +def write_summary(summary_path, lines): if not summary_path: - sys.stdout.writelines ( lines ) + sys.stdout.writelines(lines) return - with open ( summary_path, "a", encoding="utf-8" ) as f: - f.writelines ( lines ) + with open(summary_path, "a", encoding="utf-8") as f: + f.writelines(lines) def main(): - job_name = os.environ.get ( "JOB_NAME", "unit tests" ) - summary_path = os.environ.get ( "GITHUB_STEP_SUMMARY" ) + job_name = os.environ.get("JOB_NAME", "unit tests") + summary_path = os.environ.get("GITHUB_STEP_SUMMARY") - lines = [ "### JUnit results: {}\n\n".format ( job_name ) ] + lines = ["### JUnit results: {}\n\n".format(job_name)] suites = None - if not os.path.exists ( RESULTS_PATH ): - lines.append ( "_No {} was produced (the build or test run likely failed before it could be written)._\n\n".format ( RESULTS_PATH ) ) + if not os.path.exists(RESULTS_PATH): + lines.append( + "_No {} was produced (the build or test run likely failed before it could be written)._\n\n".format( + RESULTS_PATH + ) + ) else: try: - suites = read_suites ( RESULTS_PATH ) + suites = read_suites(RESULTS_PATH) except ET.ParseError as e: - lines.append ( "_{} could not be parsed ({})._\n\n".format ( RESULTS_PATH, e ) ) + lines.append("_{} could not be parsed ({})._\n\n".format(RESULTS_PATH, e)) if suites is None: - write_summary ( summary_path, lines ) + write_summary(summary_path, lines) return - lines.append ( "| Suite | Tests | Passed | Failed | Skipped | Time (s) |\n" ) - lines.append ( "|---|---|---|---|---|---|\n" ) + lines.append("| Suite | Tests | Passed | Failed | Skipped | Time (s) |\n") + lines.append("|---|---|---|---|---|---|\n") failures = [] for suite in suites: - tests = int ( suite.get ( "tests", 0 ) ) - failed = int ( suite.get ( "failures", 0 ) ) + int ( suite.get ( "errors", 0 ) ) - skipped = int ( suite.get ( "skipped", 0 ) ) + tests = int(suite.get("tests", 0)) + failed = int(suite.get("failures", 0)) + int(suite.get("errors", 0)) + skipped = int(suite.get("skipped", 0)) passed = tests - failed - skipped status = "PASS" if failed == 0 else "FAIL" - lines.append ( - "| {} {} | {} | {} | {} | {} | {} |\n".format ( status, suite.get ( "name" ), tests, passed, failed, skipped, suite.get ( "time", "?" ) ) + lines.append( + "| {} {} | {} | {} | {} | {} | {} |\n".format( + status, + suite.get("name"), + tests, + passed, + failed, + skipped, + suite.get("time", "?"), + ) ) - for testcase in suite.findall ( "testcase" ): - node = testcase.find ( "failure" ) + for testcase in suite.findall("testcase"): + node = testcase.find("failure") if node is None: - node = testcase.find ( "error" ) + node = testcase.find("error") if node is not None: - failures.append ( ( testcase.get ( "name" ), ( node.get ( "message" ) or "" ).strip() ) ) + failures.append( + (testcase.get("name"), (node.get("message") or "").strip()) + ) if failures: - lines.append ( "\n
Failed tests\n\n" ) + lines.append("\n
Failed tests\n\n") for name, message in failures: - lines.append ( "- `{}`: {}\n".format ( name, message ) ) - lines.append ( "\n
\n" ) + lines.append("- `{}`: {}\n".format(name, message)) + lines.append("\n
\n") - lines.append ( "\n" ) + lines.append("\n") - write_summary ( summary_path, lines ) + write_summary(summary_path, lines) if __name__ == "__main__": From 383c143fc2205c73009c6526b61f47be31cc187d Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:45:40 +0700 Subject: [PATCH 13/18] Build: Auto-bump the gcovr version pin used by unit-tests.yml gcovr's version was hand-pinned with no update mechanism, unlike the other pinned dependencies. Add a bump-dependencies.yml matrix entry following the existing pattern: gcovr publishes GitHub releases (like aqt/create-dmg/jack/etc.), so reuse the same gh-release-tag lookup and extend the perl regex scan to match the "GCOVR_VERSION: " YAML pin. Not marked Changelog-worthy since gcovr is CI-only tooling, never shipped to end users -- same treatment as aqt and choco-jom. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/workflows/bump-dependencies.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/bump-dependencies.yml b/.github/workflows/bump-dependencies.yml index 2cb12fe315..c7ec715706 100644 --- a/.github/workflows/bump-dependencies.yml +++ b/.github/workflows/bump-dependencies.yml @@ -82,6 +82,11 @@ jobs: grep -oP '.*\K(?:ASIO-SDK|asiosdk)_.*(?=\.zip)' local_version_regex: (.*["\/])((?:ASIO-SDK|asiosdk)_[^"]+?)(".*|\.zip.*) + - name: gcovr + # not Changelog-worthy + get_upstream_version: GH_REPO=gcovr/gcovr gh release view --json tagName --jq .tagName | sed -re 's/^v//' + local_version_regex: (.*GCOVR_VERSION:\s*)([0-9.]+)(.*) + steps: - uses: actions/checkout@v7 with: From ebb39cebe2a05d43d44a1df47677f505ceb38553 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:45:52 +0700 Subject: [PATCH 14/18] Don't require JAMULUS_BUILD_VERSION for the autobuild scripts' setup stage mac.sh and windows.ps1 validated JAMULUS_BUILD_VERSION unconditionally at the top of the script, even though only the stages that actually read it need it: mac.sh's "build" (via mac/deploy_mac.sh, which reads the inherited env var directly for the .dmg/.pkg names) and "get-artifacts", and windows.ps1's "get-artifacts" (deploy_windows.ps1 computes its own version from source and never touches the env var, so "build" doesn't need it there). Move the check into a validate_build_version()/Get-Validated-Build-Version() helper called from just those stages, so "setup" runs without the variable set. This lets unit-tests.yml drop its JAMULUS_BUILD_VERSION: 0.0.0 dummy env, added only to satisfy the old unconditional check for a stage that never used it. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/autobuild/mac.sh | 18 ++++++++++++++---- .github/autobuild/windows.ps1 | 14 +++++++++++--- .github/workflows/unit-tests.yml | 4 ---- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/.github/autobuild/mac.sh b/.github/autobuild/mac.sh index a70cedc9e9..7e7a84817f 100755 --- a/.github/autobuild/mac.sh +++ b/.github/autobuild/mac.sh @@ -59,10 +59,16 @@ if [[ ! ${QT_VERSION:-} =~ [0-9]+\.[0-9]+\..* ]]; then echo "Environment variable QT_VERSION must be set to a valid Qt version" exit 1 fi -if [[ ! ${JAMULUS_BUILD_VERSION:-} =~ [0-9]+\.[0-9]+\.[0-9]+ ]]; then - echo "Environment variable JAMULUS_BUILD_VERSION has to be set to a valid version string" - exit 1 -fi + +# Only stages that actually consume JAMULUS_BUILD_VERSION need to validate it; +# call this at the top of those (build, get-artifacts) so e.g. "setup" can run +# without it being set. +validate_build_version() { + if [[ ! ${JAMULUS_BUILD_VERSION:-} =~ [0-9]+\.[0-9]+\.[0-9]+ ]]; then + echo "Environment variable JAMULUS_BUILD_VERSION has to be set to a valid version string" + exit 1 + fi +} setup() { if [[ -d "${QT_DIR}" ]]; then @@ -185,6 +191,8 @@ prepare_signing() { } build_app_as_dmg_installer() { + validate_build_version + # Add the qt binaries to the PATH. # The clang_64 entry can be dropped when Qt <6.2 compatibility is no longer needed. export PATH="${QT_DIR}/${QT_VERSION}/macos/bin:${QT_DIR}/${QT_VERSION}/clang_64/bin:${PATH}" @@ -198,6 +206,8 @@ build_app_as_dmg_installer() { } pass_artifact_to_job() { + validate_build_version + artifact="jamulus_${JAMULUS_BUILD_VERSION}_mac${ARTIFACT_SUFFIX:-}.dmg" echo "Moving build artifact to deploy/${artifact}" mv ./deploy/Jamulus-*installer-mac.dmg "./deploy/${artifact}" diff --git a/.github/autobuild/windows.ps1 b/.github/autobuild/windows.ps1 index 263fdc20d6..997ab84a41 100644 --- a/.github/autobuild/windows.ps1 +++ b/.github/autobuild/windows.ps1 @@ -83,10 +83,15 @@ $JackBaseUrl = "https://github.com/jackaudio/jack2-releases/releases/download/v$ $Jack64Url = $JackBaseUrl + "64-v${JackVersion}.exe" $Jack32Url = $JackBaseUrl + "32-v${JackVersion}.exe" -$JamulusVersion = $Env:JAMULUS_BUILD_VERSION -if ( $JamulusVersion -notmatch '^\d+\.\d+\.\d+.*' ) +# Only stages that actually consume JAMULUS_BUILD_VERSION need to validate it; +# call this from within those (currently just get-artifacts) so "-Stage setup" +# can run without it being set. +Function Validate-Build-Version { - throw "Environment variable JAMULUS_BUILD_VERSION has to be set to a valid version string" + if ( $Env:JAMULUS_BUILD_VERSION -notmatch '^\d+\.\d+\.\d+.*' ) + { + throw "Environment variable JAMULUS_BUILD_VERSION has to be set to a valid version string" + } } # Download dependency to cache directory @@ -247,6 +252,9 @@ Function Build-App-With-Installer Function Pass-Artifact-to-Job { + Validate-Build-Version + $JamulusVersion = $Env:JAMULUS_BUILD_VERSION + # Add $BuildOption as artifact file name suffix. Shorten "jackonwindows" to just "jack": $ArtifactSuffix = switch -Regex ( $BuildOption ) { diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index f42a3775b0..efcf24e268 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -136,8 +136,6 @@ jobs: # besides qtbase). - name: Install Qt (macOS, via autobuild's mac.sh) if: runner.os == 'macOS' - env: - JAMULUS_BUILD_VERSION: 0.0.0 # required by the script's validation; unused by "setup" run: ./.github/autobuild/mac.sh setup - name: Add Qt to PATH (macOS) @@ -154,8 +152,6 @@ jobs: - name: Install Qt (Windows, via autobuild's windows.ps1) if: runner.os == 'Windows' shell: pwsh - env: - JAMULUS_BUILD_VERSION: 0.0.0 # required by the script's validation; unused by "setup" run: powershell .\.github\autobuild\windows.ps1 -Stage setup - name: Add Qt to PATH (Windows) From 0df93f2f46898e648aeeed79b415617ee9070085 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 03:04:59 +0700 Subject: [PATCH 15/18] Never grow the frame in TruncateBy A negative iBytes used to enlarge the frame via Size() - iBytes, which contradicts the helper's purpose and could hide mistakes when crafting mutation inputs; it is a no-op now. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- src/test/protocoltester.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/protocoltester.h b/src/test/protocoltester.h index f61a706225..050d1f91dd 100644 --- a/src/test/protocoltester.h +++ b/src/test/protocoltester.h @@ -111,9 +111,13 @@ class CProtocolTester */ // chops iBytes off the end of vecbyFrame, e.g. to cut into the CRC or the - // body; clamped at 0 so an oversize iBytes yields an empty frame instead - // of a negative resize (which wraps to a huge size_t). - static void TruncateBy ( CVector& vecbyFrame, const int iBytes ) { vecbyFrame.resize ( std::max ( vecbyFrame.Size() - iBytes, 0 ) ); } + // body; clamped so it can never grow the frame: a negative iBytes is a + // no-op, an oversize iBytes yields an empty frame (instead of a negative + // resize, which wraps to a huge size_t). + static void TruncateBy ( CVector& vecbyFrame, const int iBytes ) + { + vecbyFrame.resize ( std::max ( vecbyFrame.Size() - std::max ( iBytes, 0 ), 0 ) ); + } // flips every bit of the trailing CRC byte so the checksum no longer // matches the header plus body From 3b4612ae73f1ae3c99ee5c321a968fe4625d8895 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:41:23 +0700 Subject: [PATCH 16/18] Parse the Qt version from autobuild.yml instead of duplicating it autobuild.yml's macOS main build entry becomes the single source of truth; a step extracts QT_VERSION from it (failing loudly if the parse does not yield exactly one plausible version) instead of pinning a copy here that could drift. Per PR review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/workflows/unit-tests.yml | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index efcf24e268..9ae2026594 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -19,10 +19,6 @@ permissions: contents: read env: - # Must match autobuild.yml's macOS/Windows QT_VERSION -- Qt setup below - # reuses those jobs' scripts and cache. - QT_VERSION: 6.10.2 - # gcovr version used by the coverage job below, pinned for reproducible output. GCOVR_VERSION: 8.6 @@ -93,6 +89,23 @@ jobs: - name: Checkout code uses: actions/checkout@v7 + # Single source of truth for the Qt version is autobuild.yml's macOS + # main build entry -- parsed here instead of duplicating the number. + # The Qt setup below reuses that workflow's scripts and cache. + - name: Determine Qt version from autobuild.yml + if: runner.os != 'Linux' + shell: bash + run: | + QT_VERSION="$(grep -E 'base_command:.*QT_VERSION=.*mac\.sh' .github/workflows/autobuild.yml \ + | grep -v ARTIFACT_SUFFIX \ + | sed -Ee 's/.*QT_VERSION=([0-9.]+).*/\1/' || true)" + if [[ ! "$QT_VERSION" =~ ^[0-9]+(\.[0-9]+)+$ ]]; then + echo "Could not determine a unique QT_VERSION from autobuild.yml (got: '$QT_VERSION')" >&2 + exit 1 + fi + echo "Using Qt version $QT_VERSION (from autobuild.yml)" + echo "QT_VERSION=$QT_VERSION" >> "$GITHUB_ENV" + - name: Install Qt (Linux distro packages) if: runner.os == 'Linux' run: | From bc226653719a4fecfad471848d6519e7debcfdc2 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:16:43 +0700 Subject: [PATCH 17/18] Move test files from src/test/ to test/ Requested in PR review; tests intuitively belong at the top level, not nested under src/. Fixes up test.pro's source/header paths, the CI workflow and script paths, and extends the sidecar tooling that lists source directories by name: the copyright-notice script's find list and Jamulus.pro's clang-format source regex. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- .github/scripts/run-unit-tests.py | 4 ++-- .github/workflows/unit-tests.yml | 3 ++- Jamulus.pro | 2 +- {src/test => test}/protocoltester.h | 0 {src/test => test}/test.pro | 14 +++++++------- {src/test => test}/tst_protocol.cpp | 0 tools/update-copyright-notices.sh | 2 +- 7 files changed, 13 insertions(+), 12 deletions(-) rename {src/test => test}/protocoltester.h (100%) rename {src/test => test}/test.pro (84%) rename {src/test => test}/tst_protocol.cpp (100%) diff --git a/.github/scripts/run-unit-tests.py b/.github/scripts/run-unit-tests.py index 1235286061..84d5ccc98c 100755 --- a/.github/scripts/run-unit-tests.py +++ b/.github/scripts/run-unit-tests.py @@ -110,7 +110,7 @@ def cmd_build(): run( [ qmake_bin, - "..\\src\\test\\test.pro", + "..\\test\\test.pro", "CONFIG-=debug_and_release", "CONFIG+=release", "DESTDIR=.", @@ -120,7 +120,7 @@ def cmd_build(): ) run(["nmake"], cwd=BUILD_DIR) else: - run([qmake_bin, "../src/test/test.pro"] + qmake_extra_args(), cwd=BUILD_DIR) + run([qmake_bin, "../test/test.pro"] + qmake_extra_args(), cwd=BUILD_DIR) run(["make", "-j{}".format(os.cpu_count() or 1)], cwd=BUILD_DIR) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 9ae2026594..58311c6902 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -6,12 +6,14 @@ on: branches: [ main ] paths: - 'src/**' + - 'test/**' - '.github/workflows/unit-tests.yml' - '.github/scripts/**' pull_request: branches: [ main ] paths: - 'src/**' + - 'test/**' - '.github/workflows/unit-tests.yml' - '.github/scripts/**' @@ -197,7 +199,6 @@ jobs: pipx run "gcovr==${{ env.GCOVR_VERSION }}" \ --root "$GITHUB_WORKSPACE" \ --filter "$GITHUB_WORKSPACE/src/" \ - --exclude "$GITHUB_WORKSPACE/src/test/" \ --object-directory build-test \ --html-details coverage-html/index.html \ --markdown coverage-summary.md \ diff --git a/Jamulus.pro b/Jamulus.pro index fa956ef6d5..67d8bab0f7 100644 --- a/Jamulus.pro +++ b/Jamulus.pro @@ -1191,7 +1191,7 @@ contains(CONFIG, "disable_srv_dns") { # Note: When extending the list of file extensions or when adding new code directories, # be sure to update .github/workflows/coding-style-check.yml and .clang-format-ignore as well. CLANG_FORMAT_SOURCES = $$files(*.cpp, true) $$files(*.mm, true) $$files(*.h, true) -CLANG_FORMAT_SOURCES = $$find(CLANG_FORMAT_SOURCES, ^\(android|ios|mac|linux|src|windows\)/) +CLANG_FORMAT_SOURCES = $$find(CLANG_FORMAT_SOURCES, ^\(android|ios|mac|linux|src|test|windows\)/) CLANG_FORMAT_SOURCES ~= s!^\(libs/.*/|src/res/qrc_resources\.cpp\)\S*$!!g clang_format.commands = 'clang-format -i $$CLANG_FORMAT_SOURCES' QMAKE_EXTRA_TARGETS += clang_format diff --git a/src/test/protocoltester.h b/test/protocoltester.h similarity index 100% rename from src/test/protocoltester.h rename to test/protocoltester.h diff --git a/src/test/test.pro b/test/test.pro similarity index 84% rename from src/test/test.pro rename to test/test.pro index 2507c532e5..d8ba472780 100644 --- a/src/test/test.pro +++ b/test/test.pro @@ -2,7 +2,7 @@ # # Build and run (out of tree builds are recommended): # mkdir build-test && cd build-test -# qmake ../src/test/test.pro +# qmake ../test/test.pro # make && ./jamulus-test # # "make check" is supported as well (CONFIG += testcase). @@ -40,13 +40,13 @@ win32 { DEFINES += NOMINMAX } -INCLUDEPATH += .. +INCLUDEPATH += ../src -HEADERS += ../global.h \ - ../protocol.h \ - ../util.h \ +HEADERS += ../src/global.h \ + ../src/protocol.h \ + ../src/util.h \ protocoltester.h -SOURCES += ../protocol.cpp \ - ../util.cpp \ +SOURCES += ../src/protocol.cpp \ + ../src/util.cpp \ tst_protocol.cpp diff --git a/src/test/tst_protocol.cpp b/test/tst_protocol.cpp similarity index 100% rename from src/test/tst_protocol.cpp rename to test/tst_protocol.cpp diff --git a/tools/update-copyright-notices.sh b/tools/update-copyright-notices.sh index 8b0799ea71..9f15b96d56 100755 --- a/tools/update-copyright-notices.sh +++ b/tools/update-copyright-notices.sh @@ -53,7 +53,7 @@ echo "Updating global copyright strings..." sed -re 's/(Copyright.*2[0-9]{3}-)[0-9]{4}/\1'"${YEAR}"'/g' -i src/translation/*.ts src/util.cpp src/aboutdlgbase.ui echo "Updating copyright comment headers..." -find android ios linux mac src windows tools .github -regex '.*\.\(cpp\|h\|mm\|sh\|py\|pl\)' -not -regex '\./\(\.git\|libs/\|moc_\|ui_\).*' | while read -r file; do +find android ios linux mac src test windows tools .github -regex '.*\.\(cpp\|h\|mm\|sh\|py\|pl\)' -not -regex '\./\(\.git\|libs/\|moc_\|ui_\).*' | while read -r file; do sed -re 's/((\*|#).*Copyright.*[^-][0-9]{4})(\s*-\s*\b[0-9]{4})?\s*$/\1-'"${YEAR}"'/' -i "${file}" done From 33a8972188e2cf3ce8ada324740572ca98e4eac3 Mon Sep 17 00:00:00 2001 From: "dtinth-claw[bot]" <290121326+dtinth-claw[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:31:20 +0700 Subject: [PATCH 18/18] Guard CorruptCRC against empty frames TruncateBy documents that an oversize truncation yields an empty frame, so a test can legally chain it into CorruptCRC, which indexed the last byte unconditionally. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HGdLtGxMrbExBFG3aReNAH --- test/protocoltester.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/protocoltester.h b/test/protocoltester.h index 050d1f91dd..3c72a61fc7 100644 --- a/test/protocoltester.h +++ b/test/protocoltester.h @@ -120,10 +120,14 @@ class CProtocolTester } // flips every bit of the trailing CRC byte so the checksum no longer - // matches the header plus body + // matches the header plus body; no-op on an empty frame (a state + // TruncateBy can legally produce) static void CorruptCRC ( CVector& vecbyFrame ) { - vecbyFrame[vecbyFrame.Size() - 1] = static_cast ( vecbyFrame[vecbyFrame.Size() - 1] ^ 0xFF ); + if ( vecbyFrame.Size() > 0 ) + { + vecbyFrame[vecbyFrame.Size() - 1] = static_cast ( vecbyFrame[vecbyFrame.Size() - 1] ^ 0xFF ); + } } // overwrites vecbyFrame's declared body length field, independently of the