Testing advice for alpaca-java

[Kurru]

Writing tests for the java library is rather wordy. Is it possible to improve this or is it limited by the code generator?

Specifically, this is the code to setup a single API integration using a little bit of mockito:

StockBarsResponse stockBarsResponse = new StockBarsResponse();
StockBar bar1 = new StockBar();
bar1.setTimestamp(LocalDateTime.of(2022, 2, 2, 9, 0).atZone(NewYorkBroker.TIME_ZONE));
bar1.setOpen(10.0);
bar1.setHigh(11.0);
bar1.setLow(9.0);
bar1.setClose(10.5);
bar1.setTradeCount(100L);
bar1.setVolume(100L);
bar1.setVwap(10.1);

stockBarsResponse.setBars(new ArrayList<>(ImmutableList.of(bar1, bar2)));
when(alpacaAPI.stockMarketData().getBars(eq("TWTR"),
    any(ZonedDateTime.class),
    any(ZonedDateTime.class),
    eq(10000),
    isNull(),
    eq(5),
    eq(BarTimePeriod.MINUTE),
    eq(BarAdjustment.SPLIT),
    eq(BarFeed.SIP))).thenReturn(stockBarsResponse);

Some possible improvements:

• A builder pattern for the objects, where each setter returns this to enable method chaining, though this
• Returning long, not Long for these fields to reduce casting requirements
• setBars(ArrayList<StockBar>) would be simpler to use it if it was setBars(List<StockBar>)

Looking at jsonschema2pojo, seems that generate-builders property can provide a method chaining interface. Documentation

For Long vs long, perhaps you could use the usePrimitives option? Documentation

[Kurru]

Oh, separately, when reading bars for a time period that does not have bars, then we receive an unexpected null from StockBarsResponse.getBars(). It would be expected this would return an empty list. Is this something that can be changed or documented?

the return value of 
 "net.jacobpeterson.alpaca.model.endpoint.marketdata.stock.historical.bar.StockBarsResponse.getBars()" is null

[Petersoj]

@Kurru sorry for the late response. Thanks for your feedback! Unit testing is certainly on the TODO list, but I haven't managed to tackle all of it quite yet, but this library appears to be pretty stable anyways (though unit testing is always good to have of course). Thank you for looking into this.

To address some of the possible improvements you suggest:

• Yes, jsonschema2Pojo does offer a way to generate builder patterns for POJOs (simply add generateBuilders = true after this line in the build.gradle). I will add this in the next release.
• Long (the boxed type) should generally be used because it allows for null whereas the primitive long type does not (it defaults to 0), and sometimes JSON responses return null for long-type fields. That can mean that a returned long-type in a JSON response could either be nullor 0 but how can the developer know which is which if only the primitive type is used in the POJO? This is why the boxed type is used for primitive fields.
• It would indeed be simpler to use the List interface for the setBars method, but the reason I chose ArrayList was to explicitly define the List implementation used by Gson (but perhaps I can create a (de)serialization strategy or type adapter for List fields in POJOs, I'll look into that later).

Regarding your second comment about getBars() returning null, I receive a response of {"bars":null,"symbol":"AAPL","next_page_token":null} when attempting to query an empty bar data range for AAPL in the following piece of code:

StockBarsResponse aaplBarsResponse = alpacaAPI.stockMarketData().getBars(
                "AAPL",
                ZonedDateTime.of(2021, 7, 6, 0, 0, 0, 0, ZoneId.of("America/New_York")),
                ZonedDateTime.of(2021, 7, 6, 1, 0, 0, 0, ZoneId.of("America/New_York")),
                null,
                null,
                1,
                BarTimePeriod.HOUR,
                BarAdjustment.SPLIT,
                BarFeed.SIP);
        aaplBarsResponse.getBars().forEach(System.out::println);

This code throws an NPE as expected because the value of bars is null in the returned JSON object. While I think it makes more sense for this JSON response to return an empty array [], perhaps the null value is intentional sent by Alpaca and there is some undocumented instances where an empty bar array [] and a null response mean different things (not sure if that's true, but I want this library to be as transparent as possible when transforming JSON to POJOs). So it's likely better that null is returned and this library doesn't transform null into an empty returned List. I can document this in the Javadocs in the next release.

Thanks for your feedback!

[Kurru]

Thanks for the detailed review! I find it rather frustrating when Java code returns nulls for empty lists though, as this complicates the code for processing this scenario, but I can understand your rationale for not making any assumptions about the underlying API.

Another option, could be to review how the official API implementations handle these json null in their client libraries. Perhaps there they don't always use the primitive types because they know something is not null or they don't expose nulls for lists as they know thats just how the alpaca json api library exposes empty lists.

I think the null json list to empty list could in general be acceptable expectation though. I'd personally say any API that has different behavior for empty list vs null list is a poorly defined API.

[Petersoj]

Some of this has been addressed in 10.0.0. Testing frameworks have been removed for now. Will re-open later potentially.