https://kotlinlang.org logo
#kotest-contributors
Title
# kotest-contributors
d

dr.dreigh

06/07/2020, 9:17 AM
re 826: I've got together a bit of a work in progress diff, but I'm a little stalled in places where I could do with a bit of help knowing where to put things, and how config works. This is what I have got so far on 826: https://github.com/kotest/kotest/compare/master...jaymoid:826-help-needed?expand=1 It prints diffs like:
Copy code
data class diff for com.sksamuel.kotest.eq.DataClass3
├ x: expected:<99> but was:<88>
├ y: data class diff for com.sksamuel.kotest.eq.DataClass2
│  ├ x: expected:<2> but was:<1>
│  ├ y: expected:<4.4f> but was:<3.4f>
│  └ z: data class diff for com.sksamuel.kotest.eq.DataClass1
│     ├ a: expected:<99> but was:<2>
│     └ b: expected:<7.6f> but was:<4.6f>
└ z: expected:<44.6> but was:<44.4>
I have got as far as I can on my own, there's quite a few bits in the code where I could do with a bit of help, they are marked TODO, or just HELP! 🙂 • I've added
Any?.isDataClass
function to return true the instance is a data class and if the platform supports reflection. I'm not sure I've added all the implementations in the right places, or if I actually needed them all? (Non JVM kotlin is new to me!) • Because this feature is not useful if the user decides to override
equals()
in their data class, I've added a propertyConfig value
showDetailedDataClassDiff
- however I'm not sure if a) the best way of accessing it form the
DataClassEq.kt
class? and b) if this flag should be on a per spec basis rather than project? (or both?) • I imagine that a very deeply nested data class would cause this code to stack overflow. • I'm not sure where DataClassEq.kt should live because it contains platform dependent code. Any help/suggestions/code changes/feedback would be greatly appreciated. Cheers!
s

sam

06/07/2020, 11:23 AM
Is it rebased to current master
d

dr.dreigh

06/07/2020, 12:19 PM
yeah, I pulled from upstream.
Just noticed that newline, on extensions.kt, will fix shortly [edit] fixed[/edit]
s

sam

06/07/2020, 3:00 PM
I created a PR so I could add some comments
d

dr.dreigh

06/07/2020, 4:25 PM
Cool, thanks - plenty to be getting on with 🙂
s

sam

06/07/2020, 4:26 PM
Sure is. If you can finish it next couple of days, we can include it in 4.1.0-RC1
Otherwise we can bump it to 4.2
d

dr.dreigh

06/07/2020, 4:35 PM
Right, I'll get my skates on 🙂
😂 1
Hi, I did a bit of work on this last night, if you get a min can you check it's going in the right direction please? I think I understand the reflection stuff now, I've added a function to the reflection interface to get the members declared in the primary constructor. Not sure if this was the right thing to do? If it is, I think the next step is preventing it exposing the KCallable type as non-JVM platforms don't have this and is causing some failing builds from platforms without this type.
It's now building, but not added any features to hide it behind a flag yet.
👍🏻 1
s

sam

06/09/2020, 7:14 PM
Will check tonight
d

dr.dreigh

06/10/2020, 6:53 PM
^ thanks for the feedback, I've added that config obj and had a bit of a tidy. 👍
s

sam

06/10/2020, 6:53 PM
Is it good to be merged ?
d

dr.dreigh

06/10/2020, 6:55 PM
haha - err I think it's very close 😬 let me know what you think - also happy to raise a new PR with commits squished
s

sam

06/10/2020, 7:07 PM
I can squash when merging
👍 1
d

dr.dreigh

06/11/2020, 6:51 PM
Hi, there's a couple of failing tests when run on ubuntu - they pass on my machine 🤣 They are:
Copy code
Test shouldTimeout should pass if a coroutine takes longer than the given timeout[jvm] FAILED
Test executionError[jvm] FAILED
Wondering if they are bit flakey because they are timing based? Not sure I've done anything to affect that area.
s

sam

06/11/2020, 6:54 PM
They're new, and have pretty tight timings so yeah that'll be why
d

dr.dreigh

06/11/2020, 6:58 PM
aaah ok, cool
s

sam

06/11/2020, 6:59 PM
The options you moved to the config object
did you leave typealiases in place ?
never mind I see they were just sysprops before
d

dr.dreigh

06/11/2020, 7:07 PM
ah yeah - is that ok? Thought I'd try and move those into one place
s

sam

06/11/2020, 7:14 PM
no that's good
d

dr.dreigh

06/11/2020, 7:41 PM
ah you've merged it - thanks 🙂
👍🏻 1
3 Views