Skip to content

fix: add Spotless skip flags to Maven and Gradle validation#1946

Open
mashraf-222 wants to merge 3 commits intomainfrom
cf-1080-spotless-skip
Open

fix: add Spotless skip flags to Maven and Gradle validation#1946
mashraf-222 wants to merge 3 commits intomainfrom
cf-1080-spotless-skip

Conversation

@mashraf-222
Copy link
Copy Markdown
Contributor

Problem

Instrumented test files (*__perfinstrumented.java) fail Spotless Maven/Gradle format checks. The validation skip flag lists did not include Spotless, causing build failures on projects like Apache Flink (Spotless 2.43.0), Kafka, and Beam.

Root Cause

_MAVEN_VALIDATION_SKIP_FLAGS in maven_strategy.py and _GRADLE_SKIP_VALIDATION_INIT_SCRIPT in gradle_strategy.py were missing Spotless entries. They already skipped rat, checkstyle, spotbugs, pmd, enforcer, japicmp, and errorprone — but not Spotless.

Fix

Maven: Added -Dspotless.check.skip=true and -Dspotless.apply.skip=true to _MAVEN_VALIDATION_SKIP_FLAGS.

Gradle: Added spotlessCheck, spotlessApply, spotlessJava, spotlessKotlin, spotlessScala to the task disable list in the init script.

Validation

Verified flag works on Apache Flink:

$ mvn spotless:check -pl flink-core -Dspotless.check.skip=true
[INFO] BUILD SUCCESS

Test Coverage

  • test_maven_skip_flags_include_spotless — verifies both Spotless flags present
  • test_maven_skip_flags_include_all_known_plugins — verifies all plugin families covered
  • test_gradle_skip_script_includes_spotless — verifies Gradle script disables Spotless tasks

Closes CF-1080

Instrumented test files fail Spotless format checks on projects like
Apache Flink, Kafka, and Beam. Adds -Dspotless.check.skip=true and
-Dspotless.apply.skip=true to Maven, and spotlessCheck/Apply/Java/
Kotlin/Scala task disabling to the Gradle init script.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 1, 2026

Claude finished @mashraf-222's task in 1m 39s —— View job


PR Review Summary

  • Gather context
  • Triage PR size (SMALL — 27 lines of production code)
  • Lint and typecheck — clean
  • Resolve stale threads — none found
  • Code review
  • Duplicate detection
  • Post summary

Prek Checks

All clean — no linting or formatting issues.

Code Review

gradle_strategy.py / maven_strategy.py — Spotless skipping

The Maven flags (-Dspotless.check.skip=true, -Dspotless.apply.skip=true) are the correct properties for Spotless Maven Plugin. If Spotless isn't present in the project, Maven ignores unknown -D properties — no regression risk.

The Gradle approach disables named tasks. spotlessCheck and spotlessApply are the aggregate entry points, so disabling them should be sufficient. The additional spotlessJava, spotlessKotlin, spotlessScala entries are belt-and-suspenders. One note: if a project uses Spotless for other languages (Groovy, XML, JSON, Markdown), those language-specific tasks (spotlessGroovy, spotlessXml, etc.) aren't listed — but since the aggregate tasks are disabled they won't be directly invoked, so this is low risk in practice.

javascript/support.py — Windows path normalization

os.path.relpath(...).replace("\\", "/") is a correct fix for Windows. The path is used in a JavaScript import string, where backslashes are escape characters.

javascript/test_runner.py — Windows path normalization

d.replace(chr(92), '/') works, but d.replace("\\", "/") would be more idiomatic Python and immediately readable. Fix this → Minor style nit — not blocking.

project_root.as_posix() is the right call here.

verification/coverage_utils.py — POSIX path matching

Correct fix. Jest/Vitest coverage JSON keys always use forward slashes regardless of OS. Using source_code_path.resolve().as_posix() and source_code_path.as_posix() for comparison is the right approach.

Note: The endswith(source_relative_posix) check is pre-existing logic. The fix correctly normalizes the input to POSIX format to match what Jest emits, which is the right scope of change here.

test_run_and_parse.py — CV threshold relaxed from 5% to 15%

Reasonable given the explanation (Windows CI scheduler granularity of ~15ms for 10ms target sleep). The mean accuracy check (±5%) still provides a meaningful correctness guard.

test_jest_typescript_config_bug.py — timeout 5s → 30s

Appropriate for CI environments where Node.js startup can be slow.

Duplicate Detection

No duplicates detected. The Spotless skip logic is additive to existing plugin skip lists; the Windows path fixes are localized to the relevant JS modules.

Test Coverage

New tests in test_build_tools.py cover the added Maven/Gradle Spotless flags directly. Coverage is appropriate for SMALL PR scope.


Last updated: 2026-04-08

HeshamHM28
HeshamHM28 previously approved these changes Apr 7, 2026
"-Denforcer.skip=true",
"-Djapicmp.skip=true",
"-Derrorprone.skip=true",
"-Dspotless.check.skip=true",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if spotless does not exist, does this command still work correctly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it works correctly. If Spotless isn't in the project, the skip flag is just ignored, Maven doesn't fail on unknown system properties.

misrasaurabh1
misrasaurabh1 previously approved these changes Apr 7, 2026
Normalize paths to forward slashes in JS/TS code generation and coverage
parsing — backslashes are escape chars in JavaScript strings and cause
silent corruption on Windows. Also relax timing test thresholds for CI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mashraf-222 mashraf-222 dismissed stale reviews from misrasaurabh1 and HeshamHM28 via 4c70a21 April 8, 2026 00:15
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