Convert from groovy to junit #1189
Conversation
|
|
cssjessica
requested review from
DennisHeimbigner and
haileyajohnson
as code owners
cdm/core/src/test/java/ucar/nc2/ft/point/remote/TestPointStream.java
Outdated
Show resolved
Hide resolved
|
|
||
| File outFile = temporaryFolder.newFile(); | ||
| try (FeatureDatasetPoint fdPoint = | ||
| (FeatureDatasetPoint) FeatureDatasetFactoryManager.open(FeatureType.ANY_POINT, location, null)) { |
There was a problem hiding this comment.
This method is deprecated, the new method signature takes and error log formatter (which can just be null for the sake of using the current API). This test also then won't throw the NoFactoryFoundException
There was a problem hiding this comment.
The method was updated.
cdm/core/src/test/java/ucar/nc2/ft/point/remote/TestPointStream.java
Outdated
Show resolved
Hide resolved
cdm/core/src/test/java/ucar/nc2/ft/point/remote/TestPointStream.java
Outdated
Show resolved
Hide resolved
| // Ignore this class's tests if NetCDF-4 isn't present. | ||
| // We're using setup() because it shows these tests as being ignored. | ||
| // setupSpec() shows them as *non-existent*, which is not what we want. | ||
| Assume.assumeTrue("NetCDF-4 C library not present.", Nc4Iosp.isClibraryPresent()); |
There was a problem hiding this comment.
I think we still need to check that the C library is present for these tests. You can follow what this test does: https://github.com/Unidata/tds/blob/13017bd6dfd9f5ae270bf2e524d8d7340592742b/tds/src/test/java/thredds/server/ncss/view/dsg/DsgSubsetWriterTest.java#L47
There was a problem hiding this comment.
If I understand the comment correctly they wanted the tests to be ignored if the C Library isn't present. But these tests just test that no exception is thrown when a method is called, which should be the case whether or not the library exists, so I don't think that assumption is really needed
There was a problem hiding this comment.
wait, what are these even meant to be testing?
There was a problem hiding this comment.
My understanding was that at some point if you didn't have the netcdf-c library present then calling these functions caused a null pointer exception. Then they made these tests and added a null check to fix that. So they only check that there is no null pointer exception thrown, but yeah not the most useful tests of all time 😆
There was a problem hiding this comment.
lol well now they don't do anything cuz the functions return right away 🤷
There was a problem hiding this comment.
yeah it's not a super useful test... do you still want to check for the c library or leave as is?
There was a problem hiding this comment.
We ended up adding the check for the C library
Description of Changes
The following spock groovy test were converted to Junit: PointStreamSpec and Nc4IospSpec from Issue #1188.
PR Checklist
until ready for review