When using test builders, what’s the story about p...

# apollo-kotlin

[Stylianos Gakis]

When using test builders, what’s the story about populating a field which is of a type that I have a custom type mapper for? Specifically, I am building an object that at some point has a java.time.LocalDate in there, and I am using the JavaLocalDateAdapter specified in my apollo {} configuration in gradle.
In the test builder however it’s expecting a String, and I assume it will then try to use that Adapter to map it to the object. So should I have to do something like this on these cases:
(For an object that has a field called “activeFrom”)

this.activeFrom = JavaLocalDateAdapter.toJsonString(
    LocalDate.of(2021, 4, 11), // I guess no need to pass anything for customScalarAdapters here
)

And then this line inside build should take are of it right? "activeFrom" to resolve("activeFrom", LocalDate.type, emptyList()),

And a couple of thoughts if I assume that this is the way to do it. I’d of course optimally like to be able to just give it the object itself, but maybe this isn’t possible with the way the test builders are made. Maybe some convenience function could be generated that does this for us?

[mbonnin]

The story isn't great...

[mbonnin]

It's strings all the way down at the moment

[mbonnin]

As you found out

[mbonnin]

I guess we could make it use the kotlin classes instead.

[Stylianos Gakis]

Alright that’s fine as long as the snippet I suggested above is what I am supposed to do, is it? If so, I can work with it for now.

[mbonnin]

Indeed, that's the way to do it

[mbonnin]

You can just pass CustomScalarAdapters.EMPTY

[mbonnin]

That's another small inconsistency in the Adapter API. We're using the same interface for 2 different things

[mbonnin]

Leaf (==sccalar) types don't need a CustomScalarAdapters instance to serialize/deserialize

[Stylianos Gakis]

Yeah optimally I think it’d be best if somehow we could simply pass the type it’s supposed to be without the ceremony.
And luckily it’s already defined as a default param, I just pointed it out to make sure ^_^

[mbonnin]

Yea, I think it makes sense

[mbonnin]

String is/was the minimum common denominator

[Stylianos Gakis]

What two things are you referring to here?

[mbonnin]

Not 100% sure the terminology but Adapter currently takes a CustomScalarAdapter parameter that's not always useful
• For compound types it is useful in order to know how to serialize/deserialize fields
• For scalar types, it is never used

[mbonnin]

It exposes some internal details to scalar types implementers

[mbonnin]

If you're an user and want your own scalar type, you should just care about
fromJson(jsonReader: JsonReader) , not fromJson(jsonReader: JsonReader, customScalarAdapters: CustomScalarAdapters)

[mbonnin]

Not the end of the world I guess. Especially, it's useful that we have a single Adapter interface but it's something that has been itching me for a while

[Stylianos Gakis]

Ah, of course, that makes sense. I guess that’s not the end of the world though, it seems pretty harmless to just ignore that parameter for those cases.

[Stylianos Gakis]

This is more of a “future improvements” thing I’d guess 😄

[mbonnin]

Yea, Adapter<> is sooo ubiquitous I don't want to touch it too much

[mbonnin]

The test builders story needs to be improved though. Maybe we can do something before you change all your 3000+ tests...

[Stylianos Gakis]

Ahahaha I am still trying to migrate stuff, while trying not to use the test builders to avoid a huge refactor and then another one if the test builders settle on a different API.
Luckily it’s not that many tests, more like 100+, but we’re constructing most of our Data types manually with custom builders which that thing is the one taking the thousands of lines in the end. But I always keep my questions coming as you know 😅