Skip to content

[Fix] Java asserts removal #1501

Merged
HeshamHM28 merged 4 commits intoomni-javafrom
fix/java/asserts
Feb 17, 2026
Merged

[Fix] Java asserts removal #1501
HeshamHM28 merged 4 commits intoomni-javafrom
fix/java/asserts

Conversation

@HeshamHM28
Copy link
Contributor

No description provided.

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Feb 17, 2026

⚡️ Codeflash found optimizations for this PR

📄 20% (0.20x) speedup for JavaAssertTransformer._generate_replacement in codeflash/languages/java/remove_asserts.py

⏱️ Runtime : 1.01 milliseconds 841 microseconds (best of 250 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch fix/java/asserts).

Static Badge

@mashraf-222
Copy link
Contributor

E2E Validation Review

What was tested

Checked out branch fix/java/asserts, built the codeflash-java-runtime JAR, and ran E2E Java optimizations against the default test project (code_to_optimize/java/).

Steps

  1. Built codeflash-java-runtime JAR: mvn clean package -q -DskipTestspassed
  2. Built Java test project: mvn clean installpassed (after removing stale instrumented files from prior runs)
  3. Ran Java test project tests: mvn testpassed
  4. Ran PR-related unit tests:
    pytest tests/test_java_assertion_removal.py tests/test_languages/test_java/test_remove_asserts.py tests/test_languages/test_java/test_instrumentation.py -v
    
    Result: 191 passed, 3 failed
  5. Ran full E2E optimization: uv run codeflash --all --no-pr --verbose
    • Found 79 functions to optimize
    • Successfully optimized StringUtils.capitalizeWords (332% speedup, reviewer assessment: High, marked as success)
    • Processed reverseString, isPalindrome, countWords (optimization candidates found, tested)
    • Crashed on function 5 (StringUtils.countOccurrences) with InvalidJavaSyntaxErrorthis is a pre-existing issue in Java context extraction, not related to this PR

Issues Found

3 failing unit tests in test_instrumentation.py

The following tests fail because the PR updated the expected outputs for instrument_existing_test to include assertion transformation (Object _cf_result1 = calc.add(2, 2) instead of assertEquals(4, calc.add(2, 2))), but the actual instrument_existing_test function was not updated to call transform_java_assertions.

Failing tests:

  • TestInstrumentExistingTest::test_instrument_performance_mode_simple
  • TestInstrumentExistingTest::test_instrument_performance_mode_multiple_tests
  • TestInstrumentedCodeValidity::test_instrumented_code_preserves_imports

Root cause: The PR changed instrument_generated_java_test (line ~817) to always call transform_java_assertions, but instrument_existing_test (line ~314) was not updated with the same call. The test expectations were updated to assume assertions would be transformed in existing tests too, but the implementation doesn't match.

Fix needed: Either:

  • (a) Add transform_java_assertions call inside instrument_existing_test to match the updated test expectations, or
  • (b) Revert the test expectation changes for the 3 failing tests

E2E optimization: Passed with observations

  • Fibonacci tests were processed correctly (behavior and performance instrumentation worked)
  • StringUtils.capitalizeWords optimization completed end-to-end — 332% speedup, all correctness tests passed, marked as success
  • 2116 test cases were skipped during result parsing — these were existing test FibonacciTest and StringUtilsTest cases that couldn't be resolved against the registered instrumented files. This appears to be an existing behavior, not a regression from this PR.
  • Crash on countOccurrences: InvalidJavaSyntaxError due to missing closing brace in extracted Java code — pre-existing context extraction bug, not related to this PR.

Summary

Check Status
JAR build ✅ Pass
Java project build/tests ✅ Pass
Unit tests (assertion removal + new test file) ✅ 191 passed
Unit tests (instrumentation) ❌ 3 failed — instrument_existing_test not updated
E2E optimization (Fibonacci pipeline) ✅ Pass
E2E optimization (StringUtils.capitalizeWords) ✅ Pass — 332% speedup
E2E crash (countOccurrences) ⚠️ Pre-existing bug, not PR-related

Verdict: The core assertion removal logic and generated test instrumentation work correctly. The 3 failing instrumentation tests need to be fixed before merge — either by adding transform_java_assertions to instrument_existing_test or by reverting the test expectations.

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Feb 17, 2026

⚡️ Codeflash found optimizations for this PR

📄 17% (0.17x) speedup for format_line_profile_results in codeflash/languages/java/line_profiler.py

⏱️ Runtime : 17.5 milliseconds 14.9 milliseconds (best of 85 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch fix/java/asserts).

Static Badge

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Feb 17, 2026

⚡️ Codeflash found optimizations for this PR

📄 13% (0.13x) speedup for JavaAssertTransformer._find_junit_assertions in codeflash/languages/java/remove_asserts.py

⏱️ Runtime : 4.06 milliseconds 3.59 milliseconds (best of 5 runs)

A new Optimization Review has been created.

🔗 Review here

Static Badge

@HeshamHM28 HeshamHM28 merged commit 6d36536 into omni-java Feb 17, 2026
8 of 30 checks passed
@HeshamHM28 HeshamHM28 deleted the fix/java/asserts branch February 17, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants