-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Script that checks that all test oracles are compilable #51
Conversation
I've figured out the problems. It was due to how Specimin handles import statements. If a class is deleted because it is not used by the target methods, Specimin should delete the import statements for that class in the final output too. Otherwise we've got an "unfound" error. |
Professor, I've made two changes on this branch. Specimin will now:
I am not sure why the CI is failing. Please take a look. Thank you. |
When I tried running "typecheck_test_outputs.sh" in my local machine, I also got an error message without any further explanation. |
Professor @kelloggm, I have fixed the failure of CI. Please take a look. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. (since I wrote the codes for these latest commits)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that your changes remove signature checker annotations. Given the timeline we're on, I'm okay with doing that for now. However, please open an issue indicating that there is a need to add more signature checker annotations on these classes, which we can return to after the ISSTA deadline.
Once you've done that and made the one other change I requested, this is ready to merge without another review.
I expect CI to fail. @LoiNguyenCS you should check the failures here and then make PRs that correct the bugs that it exposes. Once CI on this PR is green, we can merge it and then enforce this requirement for tests going forward.