Impurity annotation #83
Conversation
…g into impurity-annotation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughMakefile: adds target Pre-merge checks and finishing touches✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learningsTip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (60)
subject-programs/annotated-jars/ClassViewer-5.0.5b.jaris excluded by!**/*.jarsubject-programs/annotated-jars/JSAP-2.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/a4j-1.0b.jaris excluded by!**/*.jarsubject-programs/annotated-jars/asm-5.0.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/bcel-5.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-cli-1.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-codec-1.9.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-collections4-4.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-compress-1.8.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-lang3-3.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-math3-3.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/commons-primitives-1.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/dcParseArgs-10.2008.jaris excluded by!**/*.jarsubject-programs/annotated-jars/easymock-3.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/fixsuite-r48.jaris excluded by!**/*.jarsubject-programs/annotated-jars/guava-16.0.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/hamcrest-core-1.3.jaris excluded by!**/*.jarsubject-programs/annotated-jars/javassist-3.19.jaris excluded by!**/*.jarsubject-programs/annotated-jars/javax.mail-1.5.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jaxen-1.1.6.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jcommander-1.35.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jdom-1.0.jaris excluded by!**/*.jarsubject-programs/annotated-jars/joda-time-2.3.jaris excluded by!**/*.jarsubject-programs/annotated-jars/jvc-1.1.jaris excluded by!**/*.jarsubject-programs/annotated-jars/nekomud-r16.jaris excluded by!**/*.jarsubject-programs/annotated-jars/pmd-core-5.2.2.jaris excluded by!**/*.jarsubject-programs/annotated-jars/sat4j-core-2.3.5.jaris excluded by!**/*.jarsubject-programs/annotated-jars/shiro-core-1.2.3.jaris excluded by!**/*.jarsubject-programs/annotated-jars/slf4j-api-1.7.12.jaris excluded by!**/*.jarsubject-programs/annotated-jars/tiny-sql-2.26.jaris excluded by!**/*.jarsubject-programs/jars/ClassViewer-5.0.5b.jaris excluded by!**/*.jarsubject-programs/jars/JSAP-2.1.jaris excluded by!**/*.jarsubject-programs/jars/a4j-1.0b.jaris excluded by!**/*.jarsubject-programs/jars/asm-5.0.1.jaris excluded by!**/*.jarsubject-programs/jars/bcel-5.2.jaris excluded by!**/*.jarsubject-programs/jars/commons-cli-1.2.jaris excluded by!**/*.jarsubject-programs/jars/commons-codec-1.9.jaris excluded by!**/*.jarsubject-programs/jars/commons-collections4-4.0.jaris excluded by!**/*.jarsubject-programs/jars/commons-compress-1.8.jaris excluded by!**/*.jarsubject-programs/jars/commons-lang3-3.0.jaris excluded by!**/*.jarsubject-programs/jars/commons-math3-3.2.jaris excluded by!**/*.jarsubject-programs/jars/commons-primitives-1.0.jaris excluded by!**/*.jarsubject-programs/jars/dcParseArgs-10.2008.jaris excluded by!**/*.jarsubject-programs/jars/easymock-3.2.jaris excluded by!**/*.jarsubject-programs/jars/fixsuite-r48.jaris excluded by!**/*.jarsubject-programs/jars/guava-16.0.1.jaris excluded by!**/*.jarsubject-programs/jars/hamcrest-core-1.3.jaris excluded by!**/*.jarsubject-programs/jars/javassist-3.19.jaris excluded by!**/*.jarsubject-programs/jars/javax.mail-1.5.1.jaris excluded by!**/*.jarsubject-programs/jars/jaxen-1.1.6.jaris excluded by!**/*.jarsubject-programs/jars/jcommander-1.35.jaris excluded by!**/*.jarsubject-programs/jars/jdom-1.0.jaris excluded by!**/*.jarsubject-programs/jars/joda-time-2.3.jaris excluded by!**/*.jarsubject-programs/jars/jvc-1.1.jaris excluded by!**/*.jarsubject-programs/jars/nekomud-r16.jaris excluded by!**/*.jarsubject-programs/jars/pmd-core-5.2.2.jaris excluded by!**/*.jarsubject-programs/jars/sat4j-core-2.3.5.jaris excluded by!**/*.jarsubject-programs/jars/shiro-core-1.2.3.jaris excluded by!**/*.jarsubject-programs/jars/slf4j-api-1.7.12.jaris excluded by!**/*.jarsubject-programs/jars/tiny-sql-2.26.jaris excluded by!**/*.jar
📒 Files selected for processing (3)
scripts/Makefile(2 hunks)scripts/mutation-randoop.sh(4 hunks)subject-programs/annotated-jars/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
scripts/mutation-randoop.sh
🔇 Additional comments (8)
scripts/Makefile (2)
67-67: Path update aligns with overall refactoring.The hamcrest jar path correctly reflects the new
jars/subdirectory structure, consistent with the mutations inmutation-randoop.shand documented in the README. No issues here.
74-80: Consider error handling trade-offs against project pattern consistency.The target lacks error handling: if
wgetfails, theunzipwill fail silently on a non-existent file, and the subsequentrm -rfcould cause problems. However, this mirrors existing download targets in the Makefile (jacoco, major, evosuite, junit)—none include error handling.The suggested fix introduces complexity not present elsewhere in the codebase. If error handling is needed, consider either:
- Applying it systematically to all download targets (requires coordinated refactoring)
- Using a simpler approach like
set -eat the target level- Accepting the current project pattern
Verify with the team whether new targets should maintain pattern consistency or adopt improved practices.
scripts/mutation-randoop.sh (3)
436-436: CHECKER_QUAL_JAR correctly added to classpath.The addition of
$CHECKER_QUAL_JARto the classpath inRANDOOP_BASE_COMMANDis correct. The jar is properly positioned afterRANDOOP_JAR, and the variable is initialized viarealpathearly (line 75), ensuring early failure if the file is missing.
333-333: Jar path update verified as consistent.All subject-programs jar references use subdirectories (annotated-jars or jars), with line 333 correctly referencing
../subject-programs/jars/jaxen-1.1.6.jar. No stray old-style references remain.
70-75: No issues found. The path is correct.The Makefile (lines 79–80) explicitly renames
checker-framework-3.50.0tochecker-frameworkafter extraction:rm -rf checker-framework && \ mv checker-framework-3.50.0 checker-frameworkThis ensures the final path
build/checker-framework/checker/dist/checker-qual.jar(used in the shell script line 75) exists as expected. The ZIP structure containschecker-framework-3.50.0/checker/dist/checker-qual.jar, which becomesbuild/checker-framework/checker/dist/checker-qual.jarafter the Makefile target executes.subject-programs/annotated-jars/README.md (3)
1-12: Documentation clearly explains annotated JARs and their purpose.The README provides excellent context on what annotated JARs are, why they matter for the GRT_FUZZING feature, and how purity qualifiers are used during test generation. The writing is clear and concise, and the connection to fuzzing behavior is well-explained.
24-28: Environment setup correctly points to the Makefile-managed Checker Framework.The environment variables on lines 25–27 correctly reference the path where the Makefile target downloads and extracts the Checker Framework. This is good alignment with the build system. Ensure this path remains stable as the Makefile evolves.
47-62: Worked example is practical and helps users get started.The a4j-1.0b example is concrete, runnable, and mirrors the step-by-step instructions above. This is a strong addition that increases the likelihood users can successfully rebuild annotated JARs without trial-and-error. No issues here.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/mutation-randoop.sh (1)
223-229: Add explicit validation for annotated-jars directory when GRT_FUZZING is enabled.The conditional SRC_JAR selection calls
realpath()directly without existence validation. If the annotated-jars directory or target JAR is missing, the script will fail with a cryptic error. This issue was flagged in the previous review and should be addressed before merge.Apply this defensive guard:
if [[ " ${RANDOOP_FEATURES[*]} " =~ "GRT_FUZZING" ]]; then # If randoop features contain "GRT_FUZZING", use annotated subject program jar. + ANNOTATED_JAR="$SCRIPT_DIR/../subject-programs/annotated-jars/$SUBJECT_PROGRAM.jar" + if [ ! -f "$ANNOTATED_JAR" ]; then + echo "${SCRIPT_NAME}: error: GRT_FUZZING enabled but annotated JAR not found: $ANNOTATED_JAR" >&2 + exit 1 + fi - SRC_JAR=$(realpath "$SCRIPT_DIR/../subject-programs/annotated-jars/$SUBJECT_PROGRAM.jar") + SRC_JAR=$(realpath "$ANNOTATED_JAR") else # Else, use the original subject program jar. SRC_JAR=$(realpath "$SCRIPT_DIR/../subject-programs/jars/$SUBJECT_PROGRAM.jar") fiThis provides clear failure messaging and prevents confusing
realpatherrors.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
scripts/Makefile(2 hunks)scripts/mutation-evosuite.sh(2 hunks)scripts/mutation-randoop.sh(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-13T23:36:38.701Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defect-randoop.sh:236-243
Timestamp: 2025-10-13T23:36:38.701Z
Learning: In Defects4J, the command `defects4j export -p cp.compile` automatically compiles the project before returning the classpath, so an explicit `defects4j compile` step is not needed when using `export -p cp.compile`.
Applied to files:
scripts/Makefile
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
scripts/Makefilescripts/mutation-randoop.shscripts/mutation-evosuite.sh
🔇 Additional comments (5)
scripts/mutation-evosuite.sh (2)
175-175: Updated JAR path location is consistent with repository structure changes.The change from
subject-programs/$SUBJECT_PROGRAM.jartosubject-programs/jars/$SUBJECT_PROGRAM.jaraligns with the directory reorganization described in the PR.
321-321: Hardcoded jaxen path updated consistently.The path change to
subject-programs/jars/jaxen-1.1.6.jaris consistent with other path updates in the file and the repository restructuring.scripts/Makefile (2)
75-81: Checker Framework build target is correctly configured.The new target correctly downloads checker-framework 3.50.0, unpacks it, and relocates it to a consistent path. The target follows the established pattern for build artifacts.
68-68: Hamcrest path update is consistent with directory restructuring.The path change aligns with the migration of subject JARs to the
jars/subdirectory.scripts/mutation-randoop.sh (1)
362-362: JAR path update is consistent with directory restructuring.The jaxen path change to
subject-programs/jars/jaxen-1.1.6.jaraligns with the overall refactoring of subject program JAR locations.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
scripts/mutation-randoop.sh(5 hunks)subject-programs/annotated-jars/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
scripts/mutation-randoop.sh
🧬 Code graph analysis (1)
scripts/mutation-randoop.sh (1)
scripts/defs.sh (1)
require_file(27-32)
🔇 Additional comments (2)
scripts/mutation-randoop.sh (2)
59-73: Good: CHECKER_QUAL_JAR validation is in place.The
require_filecheck forCHECKER_QUAL_JARat line 73 properly addresses the prior critical issue. The variable is now validated consistently with other artifact paths (RANDOOP_JAR, JACOCO_AGENT_JAR, etc.), ensuring the script fails fast with a clear error if the Makefile target hasn't been executed.
461-476: LGTM: Classpath correctly extended with CHECKER_QUAL_JAR.The addition of
$CHECKER_QUAL_JARto the RANDOOP_BASE_COMMAND classpath at line 466 is properly positioned and the variable is validated at line 73, ensuring it exists before use. The construction looks correct.
|
@mernst Yes, it is ready. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/mutation-randoop.sh(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
📚 Learning: 2025-10-14T03:30:11.765Z
Learnt from: varuniy
Repo: randoop/grt-testing PR: 64
File: scripts/defects4j-randoop.sh:332-335
Timestamp: 2025-10-14T03:30:11.765Z
Learning: In defects4j-randoop.sh, the script intentionally removes RegressionTest.java and ErrorTest.java (the driver/suite files) after test generation because run_bug_detection.pl does not expect these files. The numbered test files (RegressionTest0.java, RegressionTest1.java, ErrorTest0.java, etc.) remain in TEST_DIR and are packaged for defect detection.
Applied to files:
scripts/mutation-randoop.sh
🧬 Code graph analysis (1)
scripts/mutation-randoop.sh (1)
scripts/defs.sh (1)
require_file(27-32)
🪛 GitHub Actions: Run make under CI
scripts/mutation-randoop.sh
[error] 229-239: Make style-check failed: shell-style-check reported formatting issues in shell script (scripts/mutation-randoop.sh). Exit code 2 from 'make style-check'.
🔇 Additional comments (3)
scripts/mutation-randoop.sh (3)
59-73: LGTM: CHECKER_QUAL_JAR setup correctly implemented.The variable is properly defined with
realpath, validated withrequire_file, and integrated into the classpath. This addresses the previous critical issue.
369-369: Path update aligns with directory restructuring.The jaxen-1.1.6.jar reference is correctly updated to the new
subject-programs/jars/directory structure, consistent with other changes in this PR.
472-472: LGTM: CHECKER_QUAL_JAR correctly integrated into classpath.The JAR is properly included in the Randoop classpath alongside
RANDOOP_JAR, enabling the impurity annotation feature.
| `subject-programs/src/<subject-program>/`. | ||
| 2. **Build the plain JAR**: In the subject directory, run the build command | ||
| found in `subject-programs/README.build`. The result lands in | ||
| `subject-programs/jars/` or the subject's usual build folder. |
There was a problem hiding this comment.
@varuniy How does a user know whether the result appears in both or just one of the two places? If it appears in both places, are the two versions guaranteed to be identical?
3 participants
@varuniy @776styjsu
Should this pull request be merged?