Move jtregJdk11 test to jtreg directory.#7606
Move jtregJdk11 test to jtreg directory.#7606smillst wants to merge 4 commits intotypetools:masterfrom
Conversation
📝 WalkthroughWalkthroughThe PR removes jtreg JDK 11 test wiring from the Checker project: in Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle`:
- Line 1022: The task definition for jtregJdk11Tests is orphaned and still
references a deleted directory; locate the jtregJdk11Tests task (symbol:
jtregJdk11Tests) in checker/build.gradle and either remove the entire task
definition or retarget it to the consolidated jtregTests path (update any
file/directory references and task configuration to use jtregTests) and adjust
dependencies so nonJunitTests no longer references the removed task; ensure the
public Gradle API has no broken task names after the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21c670ea-7959-418f-ac45-b109c45e90cc
📒 Files selected for processing (4)
build.gradlechecker/jtreg/Jdk11Release8.javachecker/jtreg/issue244/MyDialog.javachecker/jtregJdk11/TEST.ROOT
💤 Files with no reviewable changes (1)
- checker/jtregJdk11/TEST.ROOT
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@checker/jtreg/Jdk11Release9.java`:
- Line 3: The test summary was incorrectly changed to "--release 9" while the
test class and run flags target release 8; revert the summary back to "--release
8" so the header, the run flags (the --release 8 invocation), and the test class
name (Jdk11Release8) all remain consistent. Locate the Jdk11Release9.java header
summary line and replace the text to match "--release 8" (or alternatively
rename the file/class and run flags to 9 if you truly intend a 9-target test),
but prefer reverting the summary to 8 to keep the test stable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f4bcae0a-2101-4c47-a5c9-e869384e2c90
📒 Files selected for processing (1)
checker/jtreg/Jdk11Release9.java
| /* | ||
| * @test | ||
| * @summary Test that --release 8 does not cause a crash. | ||
| * @summary Test that --release 9 does not cause a crash. |
There was a problem hiding this comment.
Summary now contradicts the test’s actual release target.
Line 3 says --release 9, but Line 5 and Line 8 still clearly define a release-8 test (--release 8, Jdk11Release8). Please keep these aligned; otherwise the test intent is misleading.
Suggested fix
- * `@summary` Test that --release 9 does not cause a crash.
+ * `@summary` Test that --release 8 does not cause a crash.Based on learnings: In the Checker Framework repository, when doing refactoring PRs, test files should remain stable and not be changed unless necessary.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * @summary Test that --release 9 does not cause a crash. | |
| * `@summary` Test that --release 8 does not cause a crash. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@checker/jtreg/Jdk11Release9.java` at line 3, The test summary was incorrectly
changed to "--release 9" while the test class and run flags target release 8;
revert the summary back to "--release 8" so the header, the run flags (the
--release 8 invocation), and the test class name (Jdk11Release8) all remain
consistent. Locate the Jdk11Release9.java header summary line and replace the
text to match "--release 8" (or alternatively rename the file/class and run
flags to 9 if you truly intend a 9-target test), but prefer reverting the
summary to 8 to keep the test stable.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@checker/jtreg/Jdk11Release9.java`:
- Line 8: The public class declaration currently uses the name Jdk11Release8
(class Jdk11Release8) which does not match the file name Jdk11Release9.java;
update the class name to Jdk11Release9 (or rename the file to match the class)
so the public class name matches the filename, ensuring the declaration public
class Jdk11Release9 {} replaces the existing class identifier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e06fe7b-7cff-4d15-b04d-3a4c12e908f0
📒 Files selected for processing (2)
checker/build.gradlechecker/jtreg/Jdk11Release9.java
💤 Files with no reviewable changes (1)
- checker/build.gradle
| * @compile -processor org.checkerframework.checker.interning.InterningChecker Jdk11Release9.java --release 9 | ||
| */ | ||
|
|
||
| public class Jdk11Release8 {} |
There was a problem hiding this comment.
Critical: Class name must match filename.
The class name Jdk11Release8 doesn't match the filename Jdk11Release9.java. This will cause a compilation error, as public class names must match their file names in Java.
🐛 Proposed fix
-public class Jdk11Release8 {}
+public class Jdk11Release9 {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class Jdk11Release8 {} | |
| public class Jdk11Release9 {} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@checker/jtreg/Jdk11Release9.java` at line 8, The public class declaration
currently uses the name Jdk11Release8 (class Jdk11Release8) which does not match
the file name Jdk11Release9.java; update the class name to Jdk11Release9 (or
rename the file to match the class) so the public class name matches the
filename, ensuring the declaration public class Jdk11Release9 {} replaces the
existing class identifier.
1 participant
No description provided.