-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Update to Gradle 4.0.2 #859
Conversation
@@ -27,7 +27,8 @@ public CoreSanityTest() { | |||
ManualPipelineRunner.class, | |||
SubtractionOperation.class, | |||
Main.class, | |||
CoreCommandLineHelper.class | |||
CoreCommandLineHelper.class, | |||
Pipeline.class |
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.
@JLLeitschuh Can you take a look and see why I had to ignore Pipeline to get it to build?
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.
The test wouldn't pass or the compiler would fail??
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.
Tests would not pass.
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.
Can you paste the stack trace?
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.
edu.wpi.grip.core.CoreSanityTest > testNulls FAILED
junit.framework.AssertionFailedError: Error in automated nulls test of class edu.wpi.grip.core.Pipeline
If the class is better tested explicitly, you can add testNulls() to edu.wpi.grip.core.PipelineTest
Caused by:
junit.framework.AssertionFailedError: Can't find or create a sample instance for type 'edu.wpi.grip.core.Step'; please provide one using NullPointerTester.setDefault()
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.
That makes sense. Uhhh.
In the code below this where all the setDefault
's are add:
setDefault(Step.class, new MockStep());
That should solve this problem.
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.
Thanks!
gradle.properties
Outdated
org.gradle.cache.tasks=true | ||
org.gradle.parallel=false | ||
org.gradle.configureondemand=false | ||
org.gradle.cache.tasks=false |
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.
Sure, we can do this, but why is this necessary? The gradle 4.0 default is now to run with parallel true.
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 just updated and removed the file entirely.
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 saw, thats fine. org.gradle.configureondemand=true
can speed up the build a bit but its not really that important in this build because its so small.
Codecov Report
@@ Coverage Diff @@
## master #859 +/- ##
============================================
+ Coverage 51.41% 51.43% +0.01%
Complexity 1146 1146
============================================
Files 247 247
Lines 7955 7956 +1
Branches 539 531 -8
============================================
+ Hits 4090 4092 +2
+ Misses 3679 3678 -1
Partials 186 186 |
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.
Looks great! Thanks!
closes #858