Fix all 36 skipped tests and clean up dead code (Phases 4 & 5)#29
Merged
Fix all 36 skipped tests and clean up dead code (Phases 4 & 5)#29
Conversation
…e (Phase 4) Source fixes: - phev_core: XOR-decode type byte in getType(), apply XOR in encodeMessage(), use message->length in XOROutboundMessage() for concatenated messages - phev_pipe: outputChainInputTransformer uses decoded phevMessage->XOR instead of message->ctx to detect and propagate XOR; returns decoded message when XOR is present. commandResponder inherits XOR from transformer-set message->ctx when decode finds XOR=0. - phev_service: guard against NULL ctx->pipe in jsonInputTransformer; add 'update' operation validation in validateCommand() Test fixes: - test_phev_core: remove 16 SKIPs, fix expected values for XOR-aware encoding/decoding - test_phev_pipe: remove 10 SKIPs, add connect-fail stub for waitForConnection timeout test - test_phev_service: remove 10 SKIPs, fix expected JSON output formats All 219 tests pass with 0 skips.
- Delete unused sources: msg_gcp_mqtt.c/h, msg_mqtt_paho.c/h, config.h - Remove BUILD_TRANSPORT_BACKENDS option and conditional blocks from CMakeLists.txt - Delete stale remote branches (phase2/test-migration, add-license-1, register_fix, robustxor) and local branches (phase1-3) - Update AGENTS.md: remove transport backend references, reflect 219/219 test count with 0 skips - Update TODO.md: mark Phases 4 and 5 complete
papawattu
force-pushed
the
phase4/fix-skipped-tests
branch
from
8322c1e to
51a0c01
Compare
There was a problem hiding this comment.
Pull request overview
This PR completes Phases 4 & 5 of the restructure work by eliminating all previously skipped phev test cases (now 219/219 passing) via XOR/pipe/service fixes, and then removing dead transport backend code and related build/install options.
Changes:
- Fix XOR handling across core encode/decode and pipe transformer/responder paths so encrypted messages are classified/encoded/propagated correctly.
- Unskip and correct expected bytes in
test_phev_core,test_phev_pipe, andtest_phev_serviceto validate the repaired behavior. - Remove unused MQTT/GCP transport backend sources/headers and simplify CMake + docs + ignore patterns.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_phev_service.c | Unskips service tests; adds NULL-ctx guard coverage; updates expected XORed bytes. |
| test/test_phev_pipe.c | Unskips pipe tests; adds failing connect stub to exercise timeout logic. |
| test/test_phev_core.c | Unskips core tests; updates expected commands/XOR/checksum behaviors. |
| src/phev/phev_service.c | Adds operation.update validation and guards jsonInputTransformer against NULL pipe ctx. |
| src/phev/phev_pipe.c | Adjusts XOR detection/propagation; attempts to return decoded messages for XOR inputs. |
| src/phev/phev_core.c | Fixes XOR-aware type decoding, outbound encoding XOR application, and outbound XOR length handling. |
| CMakeLists.txt | Removes BUILD_TRANSPORT_BACKENDS option and related sources/install blocks. |
| TODO.md | Marks phases complete and documents the fixes applied. |
| AGENTS.md | Updates test status and removes references to the deleted transport backends option. |
| .gitignore | Ignores build variants, tool state, and core dumps. |
| src/msg/msg_mqtt_paho.c | Deleted dead transport backend implementation. |
| src/msg/msg_gcp_mqtt.c | Deleted dead transport backend implementation. |
| include/msg/msg_mqtt_paho.h | Deleted dead transport backend header. |
| include/msg/msg_gcp_mqtt.h | Deleted dead transport backend header. |
| include/msg/config.h | Deleted stale/unused config header. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+284
to
+285
| LOG_V(APP_TAG, "END - outputChainInputTransformer (decoded)"); | ||
| return decoded; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test_phev_core(16),test_phev_pipe(10), andtest_phev_service(10). All 219 tests now pass with 0 skips.Phase 4 — Source fixes
phev_core.c:
phev_core_getType(): XOR-decode the type byte before classifyingphev_core_encodeMessage(): apply XOR encoding to serialized bytesphev_core_XOROutboundMessage()/xorDataOutbound(): usemessage->lengthinstead ofdata[1]+2so concatenated messages (e.g. start message) are fully encodedphev_pipe.c:
outputChainInputTransformer: use decodedphevMessage->XOR(notmessage->ctx) to detect and propagate XOR; return decoded message when XOR is presentcommandResponder: inherit XOR from transformer-setmessage->ctxwhen internal decode finds XOR=0phev_service.c:
jsonInputTransformer: guard against NULLctx->pipevalidateCommand(): add missing "update" operation validationPhase 5 — Cleanup
msg_gcp_mqtt.c/h,msg_mqtt_paho.c/h,config.hBUILD_TRANSPORT_BACKENDSCMake option and conditional blocksAGENTS.mdandTODO.mdTest results