-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eliminate trailing double newline in dump #192
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,4 +3,3 @@ public final class org/different/pack/BuildConfig { | |
public final fun f1 ()I | ||
public final fun getP1 ()I | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,4 +3,3 @@ public final class com/company/BuildCon { | |
public final fun f1 ()I | ||
public final fun getP1 ()I | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,3 @@ public final class mixed/MarkedPublicWithPrivateMembers { | |
public fun <init> ()V | ||
public final fun otherFun ()V | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,4 +5,3 @@ public final class foo/ClassWithProperties { | |
public final fun setBar1 (I)V | ||
public final fun setBar2 (I)V | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,3 @@ public final class com/company/subsub1/Subsub1Class { | |
public fun <init> ()V | ||
public final fun getProp ()I | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,3 @@ public final class com/company/subsub2/Subsub2Class { | |
public fun <init> ()V | ||
public final fun getProp ()I | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
public abstract interface annotation class cases/annotations/Foo : java/lang/annotation/Annotation { | ||
public abstract fun i ()I | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,14 +18,13 @@ fun List<ClassBinarySignature>.dumpAndCompareWith(to: File) { | |
to.bufferedWriter().use { dump(to = it) } | ||
fail("Expected data file did not exist. Generating: $to") | ||
} else { | ||
val actual = dump(to = StringBuilder()) | ||
val actual = dump(to = StringBuilder()).toString() | ||
assertEqualsToFile(to, actual) | ||
} | ||
} | ||
|
||
private fun assertEqualsToFile(expectedFile: File, actual: CharSequence) { | ||
val actualText = actual.trimTrailingWhitespacesAndAddNewlineAtEOF() | ||
val expectedText = expectedFile.readText().trimTrailingWhitespacesAndAddNewlineAtEOF() | ||
private fun assertEqualsToFile(expectedFile: File, actualText: String) { | ||
val expectedText = expectedFile.readText() | ||
|
||
if (OVERWRITE_EXPECTED_OUTPUT && expectedText != actualText) { | ||
expectedFile.writeText(actualText) | ||
|
@@ -34,8 +33,3 @@ private fun assertEqualsToFile(expectedFile: File, actual: CharSequence) { | |
|
||
assertEquals(expectedText, actualText, "Actual data differs from file content: ${expectedFile.name}\nTo overwrite the expected API rerun with -Doverwrite.output=true parameter\n") | ||
} | ||
|
||
private fun CharSequence.trimTrailingWhitespacesAndAddNewlineAtEOF(): String = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another testing oddity. This fundamentally undermines the ability to validate the textual representation of the dump and prevented actually asserting against the behavior change in this PR. The presence or absence of trailing whitespace on a line and the presence or absence of trailing newlines are part of the output and should be validated as such. |
||
this.lineSequence().map { it.trimEnd() }.joinToString(separator = "\n").let { | ||
if (it.endsWith("\n")) it else it + "\n" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that you can add multiple dump files and have them magically joined is weird. It's only used in one test. That test should specify a combined file and this functionality should be eliminated.
Can do in a follow-up if you agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not only about dumps, and I personally find it quite convenient when it comes to constructing Gradle build scripts, like here:
binary-compatibility-validator/src/functionalTest/kotlin/kotlinx/validation/test/InputJarTest.kt
Line 18 in 218728f