API Tests #143

AustinShalit · 2017-08-12T18:23:55Z

No description provided.

AustinShalit · 2017-08-12T18:28:23Z

@JLLeitschuh Is there something I can do better with the build files?

nightpool · 2017-08-12T18:55:30Z

api/src/main/java/edu/wpi/first/shuffleboard/api/util/NetworkTableUtils.java

@@ -26,6 +26,7 @@
  private static final Pattern newMetadataPattern = Pattern.compile("/\\.");

  private NetworkTableUtils() {
+    throw new UnsupportedOperationException("This is a utility class!");


the private constructor is sufficient here.

This prevents instantiation from reflection

why do we care if someone instantiates with reflection?

It prevents instantiation from within the class. Useful from a maintainability standpoint

nightpool · 2017-08-12T18:58:42Z

build.gradle.kts

+        fun testFx(name: String, version: String = "4.0.+") =
+                create(group = "org.testfx", name = name, version = version)
+        "testCompile"(testFx(name = "testfx-core"))
+        "testCompile"(testFx(name = "testfx-junit5"))


seems unused?

Going to be used once this PR is not WIP.

nightpool · 2017-08-12T18:59:16Z

api/src/test/java/edu/wpi/first/shuffleboard/api/util/NetworkTableUtilsTest.java

+                 "false, /my.key.with.dots",
+                 "false, /my~key~with~tildes~",
+                 "false, /~my~keywithtildes",
+                 "false, /~metadata~with~tildes~"})


this line is an edge case and I don't want to decide on it either way.

AustinShalit · 2017-08-12T20:37:43Z

On hold until #144 is solved

AustinShalit · 2017-08-17T08:10:59Z

api/src/test/java/edu/wpi/first/shuffleboard/api/util/EqualityUtilsTest.java

+
+  @Test
+  public void isEqualInvalidPrimativeTest() {
+    assertThrows(UnsupportedOperationException.class, () -> EqualityUtils.isEqual(new long[0], new long[0]));


@SamCarlberg Are long arrays something EqualityUtils.isEqual() should support?

JLLeitschuh · 2017-08-17T13:29:40Z

api/src/test/java/edu/wpi/first/shuffleboard/api/util/UtilityClassTest.java

+
+  public UtilityClassTest(Class<T> clazz) {
+    this.clazz = clazz;
+  }


You could simplify this constructor by removing the constructor argument and doing this instead:

public UtilityClassTest() { this.clazz = (new TypeToken<T>(getClass()) {}).getRawType(); }

JLLeitschuh · 2017-08-17T13:30:21Z

api/src/test/java/edu/wpi/first/shuffleboard/api/util/UtilityClassTest.java

+  public void testConstructorPrivate() throws NoSuchMethodException {
+    Constructor<T> constructor = clazz.getDeclaredConstructor();
+
+    assertFalse(constructor.isAccessible());


A message would be useful here.

JLLeitschuh · 2017-08-17T13:30:32Z

api/src/test/java/edu/wpi/first/shuffleboard/api/util/UtilityClassTest.java

+      Constructor<T> constructor = clazz.getDeclaredConstructor();
+      constructor.setAccessible(true);
+      constructor.newInstance();
+    });


A message would be useful here.

JLLeitschuh · 2017-08-17T13:30:52Z

app/app.gradle.kts

+                    "-Dtestfx.robot=glass",
+                    "-Dtestfx.headless=true",
+                    "-Dprism.order=sw",
+                    "-Dprism.text=t2k"


Indentation changes?

I think my IDE and @SamCarlberg's IDE are fighting over the indentation of this file. Will #146 fix this?

No, the indentation rule isn't correctly implemented yet.
pinterest/ktlint#76

codecov-io · 2017-08-25T00:51:34Z

Codecov Report

Merging #143 into master will increase coverage by 3.5%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##             master     #143     +/-   ##
===========================================
+ Coverage     13.21%   16.71%   +3.5%     
- Complexity      187      246     +59     
===========================================
  Files           136      136             
  Lines          3549     3547      -2     
  Branches        272      272             
===========================================
+ Hits            469      593    +124     
+ Misses         3048     2923    -125     
+ Partials         32       31      -1

AustinShalit · 2017-08-25T01:49:34Z

@nightpool I'd like to merge this PR and then open another one with more progress. That way we have coverage for PRs.

JLLeitschuh · 2017-08-25T21:04:36Z

api/api.gradle.kts

+            )
+        }
+    }
+}


Why not move this logic to the root build.gradle.kts file?

JLLeitschuh

This looks good. Nice set of changes!

JLLeitschuh · 2017-08-28T18:40:36Z

plugins/base/src/test/java/edu/wpi/first/shuffleboard/plugin/base/widget/BooleanBoxTest.java

@@ -48,6 +49,7 @@ public void testColorWhenFalse() {
  }

  @Test
+  @Disabled


A reason would have been helpful here.

AustinShalit added the work in progress A pull request that is still a work-in-progress and should not be merged label Aug 12, 2017

nightpool reviewed Aug 12, 2017

View reviewed changes

nightpool suggested changes Aug 12, 2017

View reviewed changes

nightpool changed the title ~~API Tests~~ [WIP] API Tests Aug 12, 2017

AustinShalit force-pushed the tests branch 2 times, most recently from e4a29ca to 80e6b88 Compare August 17, 2017 06:01

AustinShalit commented Aug 17, 2017

View reviewed changes

AustinShalit force-pushed the tests branch 2 times, most recently from 0e3d28e to 20ba46e Compare August 17, 2017 08:17

JLLeitschuh reviewed Aug 17, 2017

View reviewed changes

AustinShalit force-pushed the tests branch 2 times, most recently from 1b233e5 to f793c0b Compare August 24, 2017 23:48

AustinShalit changed the title ~~[WIP] API Tests~~ API Tests Aug 25, 2017

JLLeitschuh reviewed Aug 25, 2017

View reviewed changes

api/api.gradle.kts Outdated

)

}

}

}

Copy link

Member

JLLeitschuh Aug 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not move this logic to the root build.gradle.kts file?

AustinShalit added 8 commits August 27, 2017 16:13

NetworkTableUtils tests

f4da951

Add tests for EqualityUtils, FxUtils, and Maps

f503a9d

NumberField Tests

c3dc9c5

Fix findbugs

cdf7298

Fix FxThreadTest logic

fb78e63

EqualityUtilsTests

a2bb7b3

Fix jacoco

9ce696c

Improve UtilityClassTest

d21b89b

AustinShalit added 2 commits August 27, 2017 16:13

AsyncProperty Tests

475c19f

Move build logic to common file

6e811ff

AustinShalit force-pushed the tests branch from 3338c18 to 6e811ff Compare August 27, 2017 20:42

Spotless

d20787f

JLLeitschuh approved these changes Aug 28, 2017

View reviewed changes

Disable failing tests

421ae56

AustinShalit merged commit ea2581c into wpilibsuite:master Aug 28, 2017

AustinShalit deleted the tests branch August 28, 2017 18:34

JLLeitschuh reviewed Aug 28, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API Tests #143

API Tests #143

AustinShalit commented Aug 12, 2017

AustinShalit commented Aug 12, 2017

nightpool Aug 12, 2017

AustinShalit Aug 12, 2017

nightpool Aug 12, 2017

SamCarlberg Aug 12, 2017

nightpool Aug 12, 2017

AustinShalit Aug 12, 2017

nightpool Aug 12, 2017

AustinShalit commented Aug 12, 2017

AustinShalit Aug 17, 2017 •

edited

Loading

JLLeitschuh Aug 17, 2017

JLLeitschuh Aug 17, 2017

JLLeitschuh Aug 17, 2017

JLLeitschuh Aug 17, 2017

AustinShalit Aug 17, 2017

JLLeitschuh Aug 17, 2017

codecov-io commented Aug 25, 2017

AustinShalit commented Aug 25, 2017

JLLeitschuh Aug 25, 2017

JLLeitschuh left a comment

JLLeitschuh Aug 28, 2017

API Tests #143

API Tests #143

Conversation

AustinShalit commented Aug 12, 2017

AustinShalit commented Aug 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AustinShalit commented Aug 12, 2017

AustinShalit Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Aug 25, 2017

Codecov Report

AustinShalit commented Aug 25, 2017

Choose a reason for hiding this comment

JLLeitschuh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AustinShalit Aug 17, 2017 •

edited

Loading