Kotlin / dokka

API documentation engine for Kotlin
https://kotl.in/dokka
Apache License 2.0
3.45k stars 412 forks source link

Improve directory comparison assertion #3907

Closed adam-enko closed 3 weeks ago

adam-enko commented 3 weeks ago

Significantly improve comparison between directories.

Currently the failure messages are too obscure, and hard to diagnose. Example: https://ge.jetbrains.com/s/vl2jgh3ivevyi/tests/task/:dokka-integration-tests:gradle:test/details/org.jetbrains.dokka.it.gradle.AndroidProjectIT/generate%20dokka%20HTML(DokkaGradleProjectRunner)%5B1%5D?top-execution=1

adam-enko commented 3 weeks ago

Ah, I forgot about binary files. I'll fix that, please wait before reviewing.

UPDATE: binary files are handled correctly now

adam-enko commented 3 weeks ago

it would be nice to see also the example of how the failure will be after this changes just to compare

Are the test assertions helpful as brief examples?

https://github.com/Kotlin/dokka/blob/9c7a32aa8139c2aa34e289a99a5ff53e97aaddb4/dokka-runners/dokka-gradle-plugin/src/test/kotlin/utils/ShouldBeADirectoryWithSameContentAsTest.kt

whyoleg commented 3 weeks ago

Are the test assertions helpful as brief examples?

Yes and no :) In the description you say Currently the failure messages are too obscure, and hard to diagnose and show the example for big project test failure, while in tests we do have small artificial project, and so I don't fully understand how the diff will look like for big project with nested files/folders etc.

The only thing that I understand which will be better is that there will be diffs. That's nice!

If that's hard to do, no problem, no need to spend time on this. But in future it will be much better if it will be possible to see the difference of such changes and if it really solves the problem it's trying to solve :). While you probably saw before and after on your own it's obvious for you what are the pros, but for me it's just comparing an apple (example in description) with potato (example in tests)

adam-enko commented 3 weeks ago

So, currently the assertion failure has a pretty file tree, but it's hard to actually see what expected:<[Change at line 44] │ │ │ ├── index.html actually means.

Does this mean a file is missing or added? Or does it differ in content?

image

In this PR the assertion failure will provide a comprehensive message for all failures. It will list the files that are unexpected, missing, or differing in content. And, if the content is different, then show what expected/actual content is.