Skip to content

feat: add E2E infrastructure testing with BATS and scheduled workflows#236

Merged
groupsky merged 35 commits into
mainfrom
feature/e2e-integration-testing
Feb 7, 2026
Merged

feat: add E2E infrastructure testing with BATS and scheduled workflows#236
groupsky merged 35 commits into
mainfrom
feature/e2e-integration-testing

Conversation

@groupsky

@groupsky groupsky commented Feb 5, 2026

Copy link
Copy Markdown
Owner

Summary

Add comprehensive infrastructure-level end-to-end integration testing using BATS (Bash Automated Testing System), Docker Compose, and the production emulator CLI with GitHub Actions scheduled workflows.

This implements real, non-mocked integration testing that can test mqtt-bridge, CLI tools, and device-profiler against emulated Modbus devices over virtual serial ports using RTU transport.

Performance Optimizations ⚡

Major performance improvements through fixing race conditions and broken detection patterns:

Test Before After Improvement
MQTT connection 10,000ms 235ms 43x faster
Virtual ports 3,013ms 23ms 131x faster 🚀
Emulator startup ~6,000ms ~350ms 17x faster
Full test suite (24 tests) ~18s 8.5s 2.1x faster 📈

Root causes fixed:

  • Docker compose path issues causing silent failures
  • --quiet flag suppressing log output needed for readiness detection
  • Timeout arithmetic bugs (iterations confused with seconds)
  • Unnecessary waits instead of event-driven checks

Architecture

Technology Stack:

  • BATS (bats-core): Shell/CLI testing framework (added as git submodule)
  • Docker Compose V2: Service orchestration (MQTT broker)
  • socat: Virtual serial port pairs
  • ya-modbus-emulator CLI: Production emulator with RTU transport
  • Mosquitto: Lightweight MQTT broker
  • nektos/act: Local GitHub Actions testing

Design Philosophy:

  • Real services over mocks
  • Simple over complex
  • Stable over cutting-edge
  • Local-first development
  • Use production tools (emulator CLI, not custom scripts)

What's Included

Test Infrastructure (tests/e2e/)

  • Complete test environment with Docker Compose
  • Setup scripts for virtual serial ports
  • Emulator config files (RTU transport with even parity)
  • Comprehensive cleanup automation with PID validation
  • MQTT broker configuration
  • BATS as git submodule (no system installation required)

BATS Test Suite (24 tests, all passing ✓)

  • Environment verification tests (14 tests)

    • Tool availability checks
    • Node.js version validation against package.json engines
    • MQTT pub/sub verification with broker log validation
    • Virtual serial port connectivity
    • Package build verification
  • Emulator functionality tests (7 tests)

    • Single device emulator
    • Multi-device emulator (XYMD1 only - OR-WE-516 removed due to parity incompatibility)
    • PID file management
    • Graceful shutdown with zombie process handling
  • Cleanup verification tests (3 tests)

Test Runner Features

# Run all tests with timing
./tests/e2e/run-tests.sh --timing

# Run specific tests with verbose output
./tests/e2e/run-tests.sh -t -v 02-emulator

# Use TAP format for CI
./tests/e2e/run-tests.sh --tap

# Show help
./tests/e2e/run-tests.sh --help

Emulator Configuration

  • Port 1 (Single Device): EX9EM energy meter on /tmp/ttyV0
  • Port 2 (Single Device): XYMD1 temperature controller on /tmp/ttyV2
  • RTU transport with even parity (matches factory defaults)
  • One emulator instance per virtual serial port
  • Production CLI (ya-modbus-emulator --config <file>)

CI/CD Integration

  • GitHub Actions scheduled workflow (daily at 2:30 AM UTC)
  • Runs only if changes in last 24 hours
  • Manual trigger support (workflow_dispatch)
  • Concurrency control to prevent resource conflicts
  • Comprehensive logging and artifact uploads on failure
  • Submodule checkout (bats-core)
  • Explicit permissions following security best practices

Documentation

  • Complete architecture design (DESIGN.md)
  • User guide with troubleshooting (README.md)
  • Helper function documentation
  • Local testing instructions
  • CI/CD integration notes
  • Clearly marked planned vs current capabilities

Critical Fixes (Post-Review)

Race Conditions & Resource Management

  • Concurrency control: Added to workflow to prevent conflicts
  • PID validation: Process ownership checked before cleanup
  • MQTT subscriber: Fixed variable expansion and broken pipe issues
  • Virtual port test: Eliminated 3s timeout with immediate data checking
  • Emulator startup: Removed --quiet flag blocking readiness detection

Configuration Fixes

  • OR-WE-516 removed: Parity incompatibility with XYMD1 (odd vs even)
  • XYMD1 humidity: Fixed unrealistic value (18% → 45% RH)
  • Docker compose paths: Corrected relative paths in tests
  • Parity settings: All configs use factory-default "even" parity

Documentation Updates

  • Test scenarios: Marked as planned examples, not current implementation
  • Prerequisites: Added missing netcat dependency
  • Performance metrics: Added timing information throughout

How to Use

Prerequisites:

# macOS
brew install socat mosquitto

# Linux
sudo apt-get install socat mosquitto-clients netcat-openbsd

# BATS is included as submodule - no installation needed!

Run Tests:

npm run build
./tests/e2e/run-tests.sh

# With timing information
./tests/e2e/run-tests.sh --timing

# Specific test file
./tests/e2e/run-tests.sh 02-emulator

Test Locally with act:

brew install act
act workflow_dispatch -W .github/workflows/e2e-tests.yml

Key Features

  • ✅ Repository-wide testing (not package-specific)
  • ✅ Real RTU protocol over virtual serial ports (no mocking)
  • ✅ Production emulator CLI (same tool end users use)
  • ✅ Simple shell-based testing with BATS
  • ✅ Easy to run locally and in CI
  • ✅ Scheduled workflows with change detection
  • ✅ Comprehensive documentation
  • ✅ BATS as git submodule (no system dependency)
  • ✅ Separate emulator per virtual port
  • ✅ Node version validated against package.json
  • ✅ Docker Compose V2 support
  • ✅ Concurrency control prevents resource conflicts
  • ✅ Factory-default parity configuration
  • ✅ Security-compliant workflow permissions
  • ✅ All 24 tests passing in 8.5 seconds
  • ✅ Test runner with --timing, --verbose, --tap flags
  • ✅ PID validation prevents killing wrong processes

Implementation Details

  • Virtual serial ports created with socat (no kernel modules needed)
  • Emulators use RTU transport over virtual ports with even parity
  • Each virtual port runs its own emulator instance
  • MQTT broker in Docker with health checks
  • Resource cleanup always executed (trap on EXIT/INT/TERM)
  • PID-based process tracking with ownership validation
  • Logs and PIDs tracked for debugging
  • Event-driven readiness detection (no arbitrary sleeps)
  • Process termination verified via zombie state detection

Files Changed

New Test Infrastructure:

  • tests/e2e/ - Complete E2E test environment
  • tests/e2e/vendor/bats-core/ - BATS submodule
  • .gitmodules - Git submodule configuration
  • .github/workflows/e2e-tests.yml - Scheduled workflow

Emulator CLI Improvements:

  • packages/emulator/src/cli.ts - Added keepalive mechanism

Configuration Updates:

  • tests/e2e/fixtures/bridge-config.json - Even parity
  • tests/e2e/fixtures/emulators/*.json - RTU transport, single devices

Test Infrastructure:

  • tests/e2e/tests/*.bats - All test files with optimizations
  • tests/e2e/tests/helpers.bash - Helper functions with PID validation
  • tests/e2e/run-tests.sh - Test runner with --timing support
  • tests/e2e/setup/*.sh - Setup and cleanup scripts

Documentation:

  • tests/e2e/DESIGN.md - Architecture and design decisions
  • tests/e2e/README.md - User guide and troubleshooting

Removed:

  • packages/mqtt-bridge/test/e2e/ - Deprecated TypeScript implementation (900 lines)
  • tests/e2e/setup/start-emulator.js - Custom script (123 lines)
  • tests/e2e/fixtures/devices/*.json - Duplicate device configs (57 lines)

Testing Checklist

  • Environment verification tests pass (14/14)
  • Emulator tests pass (7/7)
  • Cleanup tests pass (3/3)
  • Documentation complete
  • GitHub Actions workflow configured
  • RTU transport properly configured
  • BATS submodule working
  • Production emulator CLI working
  • Parity matches factory defaults
  • Race conditions fixed
  • Docker Compose V2 migration
  • Security compliance (workflow permissions)
  • Concurrency control added
  • Tests work in act (local CI testing)
  • All 24 tests passing (100%)
  • Performance optimized (2.1x faster)
  • MQTT bridge tests (infrastructure ready)
  • CLI tests (infrastructure ready)
  • Device profiler tests (infrastructure ready)

Breaking Changes

None. This is purely additive infrastructure.

Performance

  • Environment setup: ~5 seconds
  • Full test suite (24 tests): 8.5 seconds (was ~18s)
  • Individual test average: ~350ms (was ~750ms)
  • Fastest tests: 4-23ms
  • CI runtime: ~1m35s (includes Docker setup)

Future Extensions

The infrastructure is extensible for:

  • MQTT bridge integration tests
  • CLI command tests
  • Device profiler tests
  • TCP transport testing
  • Error scenario simulation
  • Performance benchmarking
  • Long-running stability tests
  • Coverage for all driver types

Comment thread .github/workflows/e2e-tests.yml Fixed
Comment thread .github/workflows/e2e-tests.yml Fixed
Comment thread .github/workflows/e2e-tests.yml Fixed
@pkg-pr-new

pkg-pr-new Bot commented Feb 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@ya-modbus/cli

npm i https://pkg.pr.new/@ya-modbus/cli@236

@ya-modbus/device-profiler

npm i https://pkg.pr.new/@ya-modbus/device-profiler@236

@ya-modbus/driver-ex9em

npm i https://pkg.pr.new/@ya-modbus/driver-ex9em@236

@ya-modbus/driver-loader

npm i https://pkg.pr.new/@ya-modbus/driver-loader@236

@ya-modbus/driver-or-we-516

npm i https://pkg.pr.new/@ya-modbus/driver-or-we-516@236

@ya-modbus/driver-sdk

npm i https://pkg.pr.new/@ya-modbus/driver-sdk@236

@ya-modbus/driver-types

npm i https://pkg.pr.new/@ya-modbus/driver-types@236

@ya-modbus/driver-xymd1

npm i https://pkg.pr.new/@ya-modbus/driver-xymd1@236

@ya-modbus/emulator

npm i https://pkg.pr.new/@ya-modbus/emulator@236

@ya-modbus/mqtt-bridge

npm i https://pkg.pr.new/@ya-modbus/mqtt-bridge@236

@ya-modbus/transport

npm i https://pkg.pr.new/@ya-modbus/transport@236

commit: 587ac12

@groupsky
groupsky force-pushed the feature/e2e-integration-testing branch from 89cd89e to 8384203 Compare February 6, 2026 21:23
@groupsky
groupsky force-pushed the feature/e2e-integration-testing branch from 3a63e81 to c752e40 Compare February 7, 2026 06:41
Add comprehensive design document for end-to-end integration testing:
- Architecture overview with virtual serial ports, emulators, and MQTT
- Three test scenarios: single device, multiple devices, concurrent operation
- Component designs for helpers (virtual ports, emulators, bridge, fixtures)
- Test data and expected MQTT messages
- Detailed test flow from setup to teardown
- Error handling strategy
- CI/CD integration plan
- Performance considerations

This design uses real emulated devices with virtual serial ports (socat) to
test the complete system without mocking core functionality.
Add VirtualPortManager class for managing virtual serial port pairs using socat:
- Create virtual port pairs with unique names
- Track socat processes for proper cleanup
- Wait for port availability with configurable timeouts
- Handle concurrent port creation
- Comprehensive error handling and cleanup

Tests:
- 17 test cases covering all functionality
- Tests for port creation, readiness checking, cleanup, and error handling
- All tests passing with proper resource cleanup

Configuration updates:
- Updated jest config to include test directory for E2E integration tests
- Updated tsconfig.lint.json to include test directory for ESLint

This component enables real end-to-end testing with emulated Modbus devices
over virtual serial ports without requiring physical hardware.
Redesign E2E testing approach based on research:
- Use BATS (bats-core) for shell/CLI testing instead of Jest
- Use Docker Compose for service orchestration
- Use nektos/act for local GitHub Actions testing
- Create tests at repository root (tests/e2e/) not package-specific
- Focus on real services, no mocking
- Keep it simple and stable

Testing stack:
- BATS: Shell/CLI testing framework
- Docker Compose: Multi-service orchestration
- socat: Virtual serial ports on host
- Mosquitto: MQTT broker
- GitHub Actions: Scheduled workflows with change detection

Test scenarios:
- MQTT bridge with single device
- MQTT bridge with multiple devices
- CLI read/write commands
- Device discovery

Design includes:
- Complete architecture and directory structure
- BATS test examples and helper functions
- Docker Compose configuration approach
- GitHub Actions workflow with scheduling
- Local testing workflow
- Error handling and cleanup strategies

This approach is simpler, more stable, and better suited for
infrastructure testing than Jest unit tests.
Create Docker Compose environment and setup scripts for E2E testing:

Docker Compose:
- Mosquitto MQTT broker with health checks
- Configuration for testing environment
- Network isolation

Setup Scripts:
- create-virtual-ports.sh: Create socat port pairs with proper logging
- cleanup.sh: Comprehensive cleanup of all resources
- start-emulator.js: Node.js script to start emulators with device configs
- run-tests.sh: Main test runner with dependency checking

Device Fixtures:
- ex9em-device1.json: Single device configuration
- xymd1-device1.json: Multi-device setup (device 1)
- or-we-516-device2.json: Multi-device setup (device 2)
- bridge-config.json: mqtt-bridge test configuration

Features:
- Colored output for better visibility
- PID file tracking for cleanup
- Health check waiting
- Timeout handling
- Error logging

All scripts are executable and ready to use.
Create BATS test suite for infrastructure-level integration testing:

Helper Functions (helpers.bash):
- wait_for_file, wait_for_port: Timeout-based waiting
- start/stop_mqtt_subscriber: MQTT message capture
- start/stop_test_emulator: Emulator lifecycle management
- is_docker_service_healthy: Docker health checks
- assert_success, assert_failure, assert_output_contains: Assertions
- assert_file_contains: File content verification
- get_project_root, clean_test_artifacts: Utilities

Test Files:
- 01-environment.bats: Environment and setup verification
  * Tool installation checks (socat, docker, node, mosquitto)
  * MQTT broker health and connectivity
  * Virtual serial port verification
  * Package build verification
- 02-emulator.bats: Emulator functionality tests
  * Single and multiple device startup
  * PID file creation
  * Graceful shutdown
  * Log file creation
- 99-cleanup.bats: Cleanup verification
  * Cleanup script execution
  * Temporary file removal

Documentation (README.md):
- Prerequisites and installation instructions
- Running tests (quick start and manual)
- Test structure and file organization
- Writing new tests with template
- Helper function reference
- Debugging and troubleshooting guide
- Performance targets
- CI/CD integration notes

Features:
- Comprehensive helper library
- Resource cleanup in teardown
- Clear test organization
- Detailed documentation
- Ready for CI/CD integration

All test files are executable and follow BATS best practices.
Create GitHub Actions workflow for infrastructure-level E2E testing:

Schedule:
- Runs daily at 2:30 AM UTC (avoids top of hour load)
- Only runs if changes in last 24 hours
- Supports manual trigger via workflow_dispatch

Jobs:
1. check-changes: Detect recent changes
   - Check last commit timestamp
   - Skip if no changes in 24 hours
   - Always run on manual trigger

2. e2e-tests: Run integration tests
   - Setup Node.js with caching
   - Install system dependencies (socat, bats, mosquitto-clients)
   - Verify installations
   - Build all packages
   - Start Docker services (MQTT broker)
   - Create virtual serial ports
   - Run BATS test suite
   - Collect comprehensive logs on failure
   - Upload artifacts (logs) with 7-day retention
   - Always cleanup resources

3. notify-on-failure: Create issue on failure
   - Creates GitHub issue on test failure
   - Includes workflow run link and commit info
   - Avoids duplicate issues (uses labels)
   - Comments on existing open issue

Features:
- Timeout protection (30 minutes)
- Comprehensive logging on failure
- Proper resource cleanup
- Artifact uploads for debugging
- Automatic issue creation
- Change detection to avoid unnecessary runs

Logging on Failure:
- Docker Compose service status
- Docker container logs
- Socat process logs
- Emulator logs
- Virtual port status
- Process listing

The workflow is designed to be stable, informative, and maintainable.
- Add bats-core as git submodule in tests/e2e/vendor/
- Update test runner to use submodule bats instead of system installation
- Update CI workflow to checkout submodules recursively
- Update README to document submodule approach
- Fix emulator script to stay running using setInterval keepalive
- Remove bats from apt-get install in CI (now using submodule)

All 23 E2E tests now pass locally without requiring system bats installation.
Remove deprecated mqtt-bridge specific E2E testing implementation:
- Delete packages/mqtt-bridge/test/e2e/ directory and all files
- Remove TypeScript virtual port manager (virtual-ports.ts)
- Remove virtual port manager tests (virtual-ports.test.ts)
- Remove deprecated DESIGN.md from mqtt-bridge
- Revert jest.config.cjs to only include src/ tests
- Revert tsconfig.lint.json to exclude test/ directory

These were superseded by the infrastructure-level E2E testing at tests/e2e/
which uses BATS for shell-based integration testing instead of Jest/TypeScript.
Split node version test into two separate tests:
- "node is installed" - verifies node exists
- "node version matches package.json engines" - validates against engines.node field
  This removes hardcoded version check and reads from package.json dynamically

Improve MQTT broker connection test:
- Renamed to "can connect and subscribe to MQTT broker"
- Now properly captures and verifies subscription output
- Confirms message was actually received by subscriber
- Adds proper timeout and cleanup handling

Test count: 23 → 24 tests (all passing)
Replace custom start-emulator.js script with built-in emulator CLI:

**Emulator CLI improvements:**
- Add keepalive mechanism using setInterval (prevents process exit)
- CLI now properly stays running until killed

**E2E test changes:**
- Create full emulator config files (transport + devices):
  - port1-single-device.json: Single ex9em device on /tmp/ttyV0
  - port2-multi-device.json: Two devices (xymd1 + or-we-516) on /tmp/ttyV2
- Update start_test_emulator() helper to use CLI with --config flag
- Each port runs a separate emulator instance (as intended)
- Update tests to use new config file format
- Remove deprecated custom start-emulator.js script

**Benefits:**
- Uses production CLI instead of test-specific script
- Better separation: one emulator per port
- Config files match emulator's expected format
- Simpler test infrastructure

All 24 E2E tests pass ✓
**Transport fix:**
- Change from "memory" to "rtu" transport in emulator configs
- Memory transport doesn't use serial ports at all (purely in-memory)
- RTU transport actually tests serial communication over virtual ports
- Add baudRate and parity settings for proper RTU configuration

**Remove redundancy:**
- Delete fixtures/devices/ directory (ex9em, xymd1, or-we-516 configs)
- Device data is already in emulator config files
- No need to maintain data in two places
- Cleaner structure: one config file per emulator instance

All 24 tests pass with RTU transport ✓
Fix stateless documentation violations and improve test reliability:

Documentation fixes:
- Remove "Future Enhancements" section from DESIGN.md (violates stateless docs policy)
- Update directory structure to match actual implementation
- Reference issue #242 for planned test scenarios
- Fix helper function signatures in README.md

GitHub Actions improvements:
- Update actions/checkout@v4 -> @v6 for consistency with CI workflow
- Update actions/setup-node@v4 -> @v6
- Add explicit permissions blocks (minimal permissions: contents:read, issues:write)

Test robustness improvements:
- Fix MQTT race condition: use loop with sleep 0.1 instead of single sleep 1
- Standardize all sleep commands to 0.1s intervals inside loops
- Convert all fixed sleeps to polling loops that check actual conditions
- Increase timeouts proportionally (timeout*10 since sleep is 10x faster)

Sleep command audit:
- wait_for_file: sleep 1 -> 0.1 (in loop)
- wait_for_port: sleep 1 -> 0.1 (in loop)
- start_mqtt_subscriber: sleep 1 -> loop with sleep 0.1
- start_test_emulator: sleep 2 -> loop with sleep 0.1
- stop_test_emulator: sleep 1 -> loop with sleep 0.1
- run-tests.sh health check: sleep 1 -> 0.1 (in loop)
- create-virtual-ports.sh: sleep 0.5 -> 0.1 (in loop)
- cleanup.sh: sleep 0.5 -> loop with sleep 0.1
- 01-environment.bats MQTT test: sleep 1 -> loop with sleep 0.1
- 01-environment.bats serial test: sleep 0.5 -> loop with sleep 0.1

All sleep commands now follow the pattern: use sleep 0.1 inside loops that check for actual conditions rather than arbitrary time delays.

References: issue #242 for remaining test scenarios
…ry delay

Replace arbitrary wait times with actual verification that the MQTT subscriber
has connected by checking mosquitto broker logs.

Changes:
- Add unique client ID to mosquitto_sub calls (bats-test-$$, bats-subscriber-$$)
- Check docker-compose logs for "New client connected...as <client_id>" message
- Remove arbitrary sleep delays after starting subscriber
- Apply to both 01-environment.bats test and start_mqtt_subscriber helper

This eliminates race conditions where the test would publish before the
subscription is fully established, making tests more reliable and faster
(no unnecessary waiting when subscription is ready quickly).
Change sleep 2 to sleep 0.1 in the Docker health check loop for consistency
with all other sleep commands in the E2E test infrastructure.
Critical fixes:
- Change parity from "none" to "even" in all configs to match EX9EM factory defaults
  This ensures tests accurately reflect real hardware behavior

Test reliability improvements:
- Fix MQTT subscription race condition by using docker compose logs --follow
- Replace arbitrary 2s emulator delay with log-based readiness check
- Fix virtual port connectivity test race by starting reader before writer
- Improve bridge cleanup to use PID files instead of broad pkill pattern

Maintenance updates:
- Migrate from docker-compose (V1) to docker compose (V2) command syntax
- Remove obsolete version field from docker-compose.yml
- Remove temporal reference from DESIGN.md (stateless documentation)

These changes eliminate race conditions, align configuration with hardware
specifications, and modernize Docker Compose usage.
Add explicit top-level permissions block to e2e-tests workflow following
GitHub security best practices. This addresses CodeQL security alerts by
setting a restrictive default (contents: read) that jobs can override as
needed.

The workflow now follows the principle of least privilege with:
- Top-level default: contents: read
- Job-level overrides: notify-on-failure adds issues: write permission

This resolves 3 CodeQL security alerts while maintaining functional
permissions for all workflow jobs.
Remove the notify-on-failure job from the E2E workflow. Failure
notifications via GitHub issues can be noisy and are better handled
through standard GitHub Actions failure emails and workflow status
monitoring.

Benefits:
- Simplifies workflow (2 jobs instead of 3)
- Reduces noise from automatic issue creation
- Failure information still available via workflow logs and artifacts
- GitHub already notifies via email for scheduled workflow failures
…dule

Fix two workflow issues:

1. Skip check-changes job entirely when manually triggered
   - Manual triggers (workflow_dispatch) should always run tests
   - No need to check for recent commits when user explicitly requests run
   - Simplifies workflow execution for manual testing

2. Use BATS binary from submodule instead of system command
   - Changed from 'bats' to './vendor/bats-core/bin/bats'
   - Ensures consistent BATS version from submodule
   - Fixes "command not found" error in act and isolated environments

These fixes enable successful local testing with act and make manual
workflow triggers more intuitive.
Tests were using paths relative to project root, but workflow runs them
from tests/e2e/ directory. This caused "file not found" failures for:
- docker-compose.yml references
- emulator binary and config files
- cleanup script paths

Fixed by making all test paths relative to tests/e2e/:
- docker-compose.yml (was tests/e2e/docker-compose.yml)
- fixtures/emulators/*.json (was tests/e2e/fixtures/...)
- ../../packages/emulator/... (was packages/emulator/...)
- setup/cleanup.sh (was tests/e2e/setup/cleanup.sh)

Test results: 22/24 passing in act (92% success rate)
All infrastructure and functional tests now work correctly.
The wait_for_port helper function uses `nc -z` to check if ports are
listening. Netcat is not installed by default in the workflow.

On Ubuntu 24.04, netcat is a virtual package requiring explicit selection
of either netcat-traditional or netcat-openbsd. Installing netcat-openbsd
as it's the modern, maintained variant.

This fixes test 8 "MQTT broker is listening on port 1883" which was
failing with "Timeout waiting for port" due to missing nc command.
After stopping the emulator, the process may briefly exist as a zombie
until it's reaped by the parent. The test was checking kill -0 immediately,
which would succeed for zombie processes still in the process table.

Added a wait loop (up to 3 seconds) to allow the zombie to be fully reaped
before verifying the process is gone. This fixes test failures in nested
Docker environments (like act) where zombie reaping timing is different.

Reference: kill -0 returns success for zombie processes because they still
have an entry in the process table waiting to be reaped.
The test now checks /proc/$PID/stat to detect zombie processes (state 'Z')
and treats them as successfully terminated, since zombies are processes
that have completed execution but haven't been reaped yet.

This fixes false failures in nested Docker environments (act) where the
zombie reaping timing is different and the parent process might not
immediately reap the terminated child.

The test now accepts both outcomes as successful:
- Process completely gone (kill -0 fails)
- Process is a zombie (terminated but not yet reaped)
RTU transport (from PR #243) maintains an open serial port via modbus-serial's
ServerSerial, which creates active handles in the event loop. This naturally
keeps the process running, making the setInterval keepalive unnecessary.

The keepalive was originally added when using memory transport, which didn't
create any active handles. With RTU transport now the default for E2E tests,
the explicit keepalive is redundant.
Add `lock: false` option when creating RTU ServerSerial to disable exclusive
port locking (TIOCEXCL). This is required for compatibility with virtual
serial ports created by socat, which don't properly reset exclusive locks
when a process crashes or exits unexpectedly.

Without this fix, the emulator fails with "Error Resource temporarily
unavailable Cannot lock port" when trying to open socat PTYs.

References:
- Fazecast/jSerialComm#159
- serialport/node-serialport#2735
- yaacov/node-modbus-serial#547

All 24 E2E tests now pass.
…atibility

Add `lock: false` to E2E emulator transport configurations to allow the
emulator to work with virtual serial ports created by socat. This is
required because socat PTYs don't properly reset exclusive locks.

The emulator defaults to `lock: true` for production use, but E2E tests
explicitly disable it for compatibility with virtual test infrastructure.
@groupsky
groupsky force-pushed the feature/e2e-integration-testing branch from d49ccb5 to 40dfe28 Compare February 7, 2026 11:41
Revert documentation comments added in previous commit. The lock option
functionality remains as implemented in #263.
Add descriptive metadata to emulator configuration files:
- port1: Single EX9EM energy meter device
- port2: Multiple devices (XYMD1 temperature controller + OR-WE-516 relay)

This makes it clearer what hardware each fixture is emulating.
…or helper

Move the process termination waiting logic from the test into the helper
function where it can be reused by all tests that stop the emulator.

Changes:
- Increase timeout from 10s to 30s to handle slower shutdowns
- Add zombie process detection - accept Z state as successful termination
- Avoid force-killing zombie processes (they're already terminated)
- Simplify the test to just call the helper and verify final state

This makes the helper more robust and reduces code duplication.
Add 'test:e2e' script to package.json for easy E2E test execution:
  npm run test:e2e

This provides a consistent interface for running all types of tests
alongside the existing unit test scripts.
This commit addresses 7 critical issues identified during code review that could cause test failures, resource conflicts, and incorrect behavior:

**Workflow & Resource Management:**
- Add concurrency control to prevent multiple workflow runs from conflicting over shared resources (virtual ports, Docker containers, PID files)
- Add PID ownership validation in cleanup to prevent killing wrong processes if PID gets reused

**Race Conditions:**
- Fix MQTT subscriber connection verification by removing broken logs --follow approach that caused variable expansion and broken pipe issues
- Improve virtual port test timing from 0.2s to 0.5s to prevent failures on slow systems

**Configuration:**
- Remove OR-WE-516 from multi-device config due to incompatible parity requirement (requires "odd", but XYMD1 requires "even")
- Fix XYMD1 humidity value from 18% RH to 45% RH for realistic testing

**Documentation:**
- Add prominent warning that test scenarios 1-4 are planned examples, not current implementation
- Add missing netcat prerequisite to README (required for port connectivity tests)

These fixes prevent flaky tests, resource conflicts, and ensure accurate documentation of current vs planned capabilities.
@groupsky

groupsky commented Feb 7, 2026

Copy link
Copy Markdown
Owner Author

Critical Issues Fixed ✅

Applied fixes for 7 critical issues identified during code review (commit 8e469b8):

1. Workflow Concurrency Control

  • Added concurrency group to prevent resource conflicts between parallel workflow runs
  • Prevents issues with virtual ports, Docker containers, and PID files

2. Configuration: OR-WE-516 Parity Mismatch

  • Removed OR-WE-516 from multi-device config (requires "odd" parity, incompatible with XYMD1's "even" parity)
  • Updated config description to reflect single device (XYMD1 only)
  • Fixed XYMD1 humidity value from 18% RH to 45% RH for realistic testing

3. MQTT Subscriber Race Condition

  • Fixed variable expansion bug in connection verification
  • Removed broken logs --follow approach that caused broken pipe
  • Simplified to reliable polling approach

4. PID File Race Condition

  • Added PID ownership validation before killing processes
  • Checks /proc/$pid/cmdline to verify process type
  • Prevents killing wrong process if PID gets reused

5. Virtual Port Test Timing

  • Increased synchronization delay from 0.2s to 0.5s
  • Improves reliability on slow systems

6. Documentation Clarity

  • Added prominent warning that test scenarios 1-4 are planned examples, not current implementation
  • Clarifies current vs future capabilities

7. Missing Prerequisites

  • Added netcat to README.md prerequisites (required for port connectivity tests)

Test Results 🎯

All 24 E2E tests passing locally:

  • ✅ 9 environment validation tests
  • ✅ 7 emulator functionality tests
  • ✅ 5 build verification tests
  • ✅ 3 cleanup verification tests

Ready for review and merge.

The test was using 'tests/e2e/docker-compose.yml' which is incorrect when run from the tests directory. Changed to 'docker-compose.yml' to match the working directory context.

Also improved grep pattern reliability by splitting into two stages:
- First grep filters to connection lines
- Second grep checks for client_id

This fix resolves the 10+ second test timeout issue - the test now completes in <2 seconds.
Add support for BATS flags to run-tests.sh:
- --timing, -t: Show timing for each test
- --verbose, -v: Show output of passing tests
- --tap: Use TAP format
- --help, -h: Show usage information

Usage examples:
  ./run-tests.sh --timing              # All tests with timing
  ./run-tests.sh -t -v 02-emulator     # Specific tests with timing + verbose
  ./run-tests.sh --tap                 # TAP format output

This makes it easy to see test performance and debug issues without manually specifying BATS flags.
The test was waiting for the full 3-second timeout even though data arrived immediately. Now it:
1. Writes test data to the port
2. Waits 0.1s for data to arrive
3. Checks if output file has content
4. Kills the reader immediately if data present
5. Otherwise waits for timeout

Result: 3013ms → 621ms (5x faster)
Refactored test to eliminate initial wait time:
- Before: Start reader → wait 0.5s → write once → check → kill
- After: Start reader → loop (write → check immediately → break if ok)

Key improvements:
1. No initial 0.5s wait for port to open
2. Write and check in tight loop
3. Exit immediately when data received
4. Typical success on first attempt (~10-20ms)

Result: 621ms → 16ms (39x faster)
Overall: 3013ms → 16ms (188x faster than original)
…ster)

Fixed the root cause: the --quiet flag was suppressing all log output, causing the grep pattern to never match. The test waited the full 5-second timeout even though the emulator started in <50ms.

Changes:
1. Removed --quiet flag to enable log output
2. Changed grep pattern to match exact message: "Emulator started successfully"
3. Removed redundant 500ms fallback sleep
4. Added warning if startup detected without log message

Results:
- Single device: 5777ms → 352ms (16x faster)
- Multiple devices: 5826ms → 248ms (23x faster)

Same pattern as MQTT test: timeout arithmetic + broken detection = wasted waiting.
@groupsky

groupsky commented Feb 7, 2026

Copy link
Copy Markdown
Owner Author

🎉 Performance Optimization Complete - All Critical Issues Resolved

Summary of All Fixes (9 commits total)

Commits addressing critical issues from review:

  1. 8e469b8 - Fixed 7 critical race conditions and configuration issues

    • Added workflow concurrency control
    • Fixed OR-WE-516 parity mismatch (removed from multi-device config)
    • Fixed MQTT subscriber connection race
    • Added PID ownership validation
    • Improved virtual port test timing
    • Marked test scenarios as planned/examples
    • Added netcat to prerequisites
  2. 5d96c66 - Fixed docker-compose.yml path (MQTT test 75x faster)

    • Changed tests/e2e/docker-compose.ymldocker-compose.yml
    • Split grep pattern for reliability
    • Result: 10,000ms → 134ms
  3. a189547 - Added --timing, --verbose, --tap flags to test runner

    • ./run-tests.sh --timing shows per-test timing
    • ./run-tests.sh --verbose shows all output
    • ./run-tests.sh --help for usage
  4. 3b0bc37 - Optimized virtual port test (5x faster)

    • Kill reader immediately after data received
    • Result: 3,013ms → 621ms
  5. 430ea63 - Refactored virtual port test (39x faster, total 188x)

    • Loop: write → check immediately → break if ok
    • No initial wait time
    • Result: 621ms → 16ms
  6. 587ac12 - Removed --quiet flag from emulator (17x faster)

    • Fixed: --quiet was suppressing log output needed for readiness detection
    • Changed grep pattern to match exact message
    • Result: ~6,000ms → ~350ms

Performance Results 🚀

Metric Before After Improvement
MQTT connection test 10,000ms 235ms 43x faster
Virtual port test 3,013ms 23ms 131x faster 🚀
Emulator startup ~6,000ms ~350ms 17x faster
Full 24-test suite ~18s 8.5s 2.1x faster 📈

Test Timing Breakdown

ok 1 socat is installed in 15ms
ok 2 docker compose is installed in 120ms
ok 3 node is installed in 14ms
ok 4 node version matches package.json engines in 53ms
ok 5 mosquitto_sub client is available in 41ms
ok 6 MQTT broker is running in 112ms
ok 7 MQTT broker is healthy in 105ms
ok 8 MQTT broker is listening on port 1883 in 10ms
ok 9 can connect and subscribe to MQTT broker in 235ms ⚡
ok 10 virtual serial port pair 1 exists in 4ms
ok 11 virtual serial port pair 2 exists in 4ms
ok 12 virtual serial ports are connected in 23ms 🚀
ok 13 project packages are built in 15ms
ok 14 emulator can be imported in 110ms
ok 15 emulator CLI binary exists in 17ms
ok 16 emulator config files exist in 20ms
ok 17 can start emulator with single device in 357ms ⚡
ok 18 can start emulator with multiple devices in 375ms
ok 19 emulator creates PID file in 258ms
ok 20 emulator can be stopped gracefully in 352ms
ok 21 emulator logs are created in 393ms
ok 22 cleanup script exists and is executable in 4ms
ok 23 cleanup script runs without errors in 272ms
ok 24 cleanup removes temporary files in 43ms

Total time: 8.5 seconds

Common Patterns Fixed

All three major slowdowns shared the same root causes:

  1. ✅ Timeout arithmetic bugs (iterations confused with seconds)
  2. ✅ Broken detection patterns (grep never matched)
  3. ✅ Unnecessary waits (sleeping instead of event-driven checks)
  4. ✅ Missing early exits (waiting full timeout)

Ready for Merge ✅

  • ✅ All 24 tests passing
  • ✅ All critical issues resolved
  • ✅ Race conditions fixed
  • ✅ Performance optimized (2.1x overall speedup)
  • ✅ Test runner enhanced with timing support
  • ✅ Documentation updated
  • ✅ CI checks passing

This PR is production-ready! 🚀

@groupsky
groupsky merged commit f333044 into main Feb 7, 2026
15 checks passed
@groupsky
groupsky deleted the feature/e2e-integration-testing branch February 7, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants