Skip to content
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

Make all Java classes in src/test/resources folder compilable by java compiler #195

Closed
daniilyar opened this issue Jul 13, 2014 · 10 comments
Assignees

Comments

@daniilyar
Copy link
Member

We have to make testinputs (src/test/resources folder) compilable to ensure that we are testing our checks only on valid Java code. So, please:

  1. Update src/test/resources to be a source folder and remove all compiler exclusions for it in Eclipse Build path.
  2. Fix all existing compile warnings in src/test/resources.
  3. Fix all failing UTs (if any).
  4. Make folder src/test/resources a source folder for Maven and Eclipse.
  5. Prevent src/test/resources testinputs from being packed into result jar during Maven build.
@isopov
Copy link
Member

isopov commented Jul 15, 2014

We should definitely ensure that src/test/resources are compilable, but we should not ship them. So we cannot merely add them as source-folder. Maybe we can do this for some profile that is not activated during release build?

@daniilyar
Copy link
Member Author

I believe we could just skip packing these folder content into the jar in pom.xml: http://stackoverflow.com/questions/21475859/in-maven-add-a-folder-as-a-source-folder-but-dont-include-it-in-source-jar

@alexkravin
Copy link
Contributor

All the files are now compilable and excluded from .jar
https://groups.google.com/forum/#!topic/sevntu-checkstyle/AfBFiHJN9Xw

@isopov
Copy link
Member

isopov commented Jul 28, 2014

Very strange change. What is the mechanics?

@isopov
Copy link
Member

isopov commented Jul 28, 2014

Also there are numerous test deletions - are all them valid?

@alexkravin
Copy link
Contributor

Several files weren't used in UT at all and some tests were checking uncompilable cases, they were deleted.

@daniilyar
Copy link
Member Author

Not all deleted code was expected to be deleted, see comments at google groups for more details. Also, that change breaks code formatting, which wasn't expected, too. We cannot allow so huge and strange update to be merged into main repo. So,

@alexkravin, please separate your update have to several pull requests - one PR per fixes for a single check. Every PR should contain update to pom.xml which forces fixed / checked testinputs to be compiled automatically by 'mvn compile' goal. This will allow code review and code fixes flexibilty.
2owners: While merging all that PRs, only updates to java files should be merged into main repo (i.e., updates to pom.xml in every PR are only for code review convenience). After all testinputs will be fixed, there will be the last PR containing update to .classpath and pom.xml files which forces src/test/resources folder to be compilable by both Maven and Eclipse.

@daniilyar daniilyar removed the task label Aug 15, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 16, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 16, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 16, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 16, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 16, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 20, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 20, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 21, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 21, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 21, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 26, 2014
daniilyar pushed a commit that referenced this issue Aug 27, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 29, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 29, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Aug 30, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
@daniilyar
Copy link
Member Author

Additional request: we need to resolve all Eclipse warnings in testinputs code or (which is better) - find the way to ignore them in Eclipse properties.

@alexkravin
Copy link
Contributor

There's an option in Java Build Path which switches on ignoring all the compiler warnings on current folder except for warnings set in Windows-Preferences-Compiler-Errors/Warnings, I suggest this way of solving, there would be few of fixes and UT and it's inputs wouldn't be changed a lot

alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 1, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 3, 2014
alexkravin added a commit to alexkravin/sevntu.checkstyle that referenced this issue Sep 3, 2014
@daniilyar
Copy link
Member Author

All related PRs merged to main repo, issue closed. Please look at commit
70fcd37
in order to see how did we forced compilation of src/test/resources folder by Maven and Eclipse.

Eclipse compiler errors were switched OFF for new classpath folder in 720d2ec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants