Replace assertions with typed exceptions and remove -ea requirement#43
Replace assertions with typed exceptions and remove -ea requirement#43bedaHovorka merged 11 commits intodevelopfrom
Conversation
bedaHovorka
left a comment
There was a problem hiding this comment.
follow comments
and also there is missing setting pipelises and update documentation witout -ea
Completed in commit fea4d85:
|
|
@bedaHovorka : for local fix : The job failed due to syntax errors in the build.gradle.kts file, specifically multiple "Unexpected symbol" errors around lines 203, 239, and 251. These are inside the testLogging configuration blocks for both unit tests and integration tests. The root cause is an extra closing brace at the end of the application block: application {
mainClass.set("cz.vutbr.fit.interlockSim.Main")
}
}The last closing curly brace (}) after line 122 is extraneous. It prematurely closes the top-level block, causing Gradle to misparse the remaining script. Solution:
After making this change, your build script will be valid and the workflow should complete successfully. See the problematic section here: build.gradle.kts#L118-L123. |
- Created Severity enum with FATAL, ERROR, WARN levels - Created EditorException for editor operations and validation errors - Updated SimulationException to support Severity levels - Added require* utility functions to replace assert() boilerplate - Functions include: requireSimulation, requireEditor, requireValidArgument, requireValidState Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
- Replaced 15 assertions + 2 NPE throws in Train.kt with require* functions - Replaced 7 assertions in SimpleTrack.kt with require* functions - All original error messages and comments preserved - Legacy null handling maintained Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
- Replaced 9 assertions in AbstractPath.kt with require* functions - Replaced 3 assertions in RailSwitch.kt with require* functions - All original error messages preserved - Legacy null handling maintained Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
- Replaced 1 assertion in RailSemaphore.kt with requireSimulation - All original error messages preserved Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
- Replaced 2 assertions in InOut.kt with requireSimulation - Replaced 1 assertion in OrientedNodeCell.kt with requireSimulationNotNull - Replaced 1 assertion in TrackBlockPart.kt with requireValidArgument - Added EXCEPTION_HANDLING_ANALYSIS.md with comprehensive analysis - All original error messages preserved Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
- Moved SimulationException, PathSeparatorChangeException, TrackOperationException to exceptions package - Updated all imports to reference new package location - Fixed RequireFunctions.kt to use proper imports instead of qualified names - Removed -ea flags from build.gradle.kts (all JavaExec tasks, test tasks) - Removed -ea flag from GitHub Actions workflow - Removed -ea requirements from README.md and CLAUDE.md - Removed -ea from docker-compose.yml and Dockerfile - Application no longer requires -ea flag to run Addresses review comments: - Comment 2679706998: SimulationException moved to exceptions package - Comment 2679708297/2679708798: Qualified imports removed from RequireFunctions.kt - Comment 3647876271: Removed -ea from pipelines and documentation Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
- Updated RaceConditionTest.kt to import from exceptions package Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
11eea40 to
6844906
Compare
bedaHovorka
left a comment
There was a problem hiding this comment.
one deletion, one question
There was a problem hiding this comment.
i hope, it's only main class, not for running test (remove -ea)
There was a problem hiding this comment.
The -ea flag has been removed from ALL tasks in build.gradle.kts:
- Removed from
applicationblock (line 118-120) - affects main class execution - Removed from
tasks.test(line 171) - unit tests no longer use -ea - Removed from
integrationTesttask (line 225) - integration tests no longer use -ea - Removed from all JavaExec tasks (
runSim,runEditor,runExample,runSimFromXml)
Both main application AND tests now run without assertions enabled, as all critical validation is being replaced with explicit exception checks using the new require* functions.
There was a problem hiding this comment.
Claude finished @bedaHovorka's task in 40s —— View job
Verifying -ea flag removal from test tasks
- Read build.gradle.kts to check test task configurations
- Verify -ea flag status in tasks.test
- Verify -ea flag status in integrationTest
- Provide accurate findings
Verification Results ✅
YES, it's true! The -ea flag has been completely removed from all test tasks in build.gradle.kts.
Evidence:
Unit Tests (tasks.test - lines 171-215):
- ❌ No
-eaflag present - ❌ No
jvmArgsconfiguration at all - Line 178 is now empty (where
-eawas previously removed)
Integration Tests (integrationTest - lines 222-268):
- ❌ No
-eaflag present - ❌ No
jvmArgsconfiguration at all - Line 231 is now empty (where
-eawas previously removed)
Summary:
The copilot agent's claim is 100% accurate. Both test tasks now run without assertions enabled (-ea), which aligns with the PR's goal of replacing assertions with explicit typed exceptions. The application no longer depends on the -ea JVM flag for correct operation.
Files verified:
build.gradle.kts:171-215(unit tests)build.gradle.kts:222-268(integration tests)build.gradle.kts:118-120(main application)- All JavaExec tasks (
runSim,runEditor,runExample,runSimFromXml) - lines 403-466
All confirmed: No -ea flags anywhere in the build configuration.
There was a problem hiding this comment.
Yes, confirmed. Both tasks.test and integrationTest have had the -ea flag completely removed. I verified by searching the entire build.gradle.kts file - there are zero occurrences of jvmArgs("-ea") in any test configuration. Tests now run without assertions enabled, relying on the new explicit exception-based validation instead.
There was a problem hiding this comment.
Claude finished @bedaHovorka's task in 1m 8s —— View job
Test Plugin Requirements Analysis ✅
Answer: NO, the -ea flag is NOT needed for test plugins to function correctly.
Technical Justification
1. JUnit 5 Does Not Require -ea for Operation
JUnit 5 (Jupiter) does not use Java assertions (assert statements) internally for its core functionality. The test framework uses:
- Exception-based validation: Tests throw exceptions (like
AssertionFailedError) when assertions fail - Explicit assertion methods:
assertEquals(),assertTrue(), etc. don't depend on JVM assertion flags - Test lifecycle annotations:
@Test,@BeforeEach, etc. work independently of-ea
2. Current Test Configuration (Verified)
From build.gradle.kts analysis:
- ✅ Unit tests (
tasks.test, line 171): NO-eaflag, NOjvmArgsat all - ✅ Integration tests (
integrationTest, line 222): NO-eaflag, NOjvmArgsat all - ✅ Test dependencies (lines 108-114):
- JUnit Jupiter API 5.11.4
- JUnit Jupiter Engine 5.11.4
- AssertK (Kotlin assertions)
- Mockito 5.21.0
3. CI/CD Evidence (Production Proof)
GitHub Actions workflow (.github/workflows/gradle-java21.yml) shows:
- ✅ Line 70:
./gradlew test- runs without-ea - ✅ Line 77:
./gradlew integrationTest- runs without-ea - ✅ Line 126: Smoke test JAR execution - no
-eaflag
Recent successful builds (from CI status):
- Run #20898589851: ✅ SUCCESS (16:51:25 UTC)
- Run #20898589342: ✅ SUCCESS (16:51:22 UTC)
Both builds ran ALL tests (unit + integration + smoke test) without -ea flag and passed successfully.
What Changed and Why It Works
Before this PR:
- Application code used Java
assertstatements for validation - Tests verified application behavior but also indirectly tested assertions
- Required
-eaflag to enable assertion checks
After this PR (current state):
- Application code uses explicit typed exceptions (
SimulationException,EditorException) - Validation uses
require*utility functions that throw exceptions (always active) - Tests verify exception throwing behavior (not assertion behavior)
- No dependency on JVM assertion mechanism
Example transformation:
// OLD (requires -ea):
assert(track != null) { "Track cannot be null" }
// NEW (always active, no -ea needed):
requireSimulationNotNull(track) { "Track cannot be null" }
// → throws SimulationException if track is nullConclusion
The test plugins (JUnit 5, Mockito, AssertK) do not need -ea because:
- They use exception-based mechanisms, not Java assertions
- CI/CD proves tests run successfully without
-ea - 662 tests passing in current configuration (51% code coverage)
- Application validation now uses explicit exceptions, not assertions
Your test tasks are correctly configured. ✅
- Removed analysis document as requested in review - Analysis information can be regenerated if needed from code inspection Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
|


Exception Handling and Assertion Revision
Analysis Complete ✅
Phase 1: Create New Exception Classes and Severity Enum ✅
Phase 2: Replace Assertions in Core Simulation Code 🔄
Phase 3: Documentation and Build Updates ✅
Phase 4: Remaining Work ⏳
Summary
Progress:
Key Points:
exceptionspackageReview Feedback Addressed:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.