mirrors/Unciv
1
0
Fork 0
mirror of https://github.com/yairm210/Unciv.git synced 2025-07-30 14:48:56 +07:00
Code Issues Packages Projects Releases Wiki Activity

Fix test logging not using format specifiers for arbitrary text (#7073)

Browse Source
This commit is contained in:
Timo T
2022-06-05 16:57:16 +02:00
committed by GitHub
parent a7e8b1a252
commit 868e551ba9
4 changed files with 38 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
core/src/com/unciv/ui/utils/Log.kt
View File

@ -34,7 +34,9 @@ object Log {
}
/**
* Logs the given message by [java.lang.String.format]ting them with an optional list of [params].
* Logs the given message by [String.format][java.lang.String.format]ting them with an optional list of [params]. Since this uses
* [format specifiers][java.util.Formatter], make sure your message only contains `%` characters correctly used as [format specifiers][java.util.Formatter]
* and never do manual string concatenation, only use [params] to log arbitrary text that could contain `%` characters.
*
* Only actually does something when logging is enabled.
*
@ -48,7 +50,9 @@ object Log {
}
/**
* Logs the given message by [java.lang.String.format]ting them with an optional list of [params].
* Logs the given message by [String.format][java.lang.String.format]ting them with an optional list of [params]. Since this uses
* [format specifiers][java.util.Formatter], make sure your message only contains `%` characters correctly used as [format specifiers][java.util.Formatter]
* and never do manual string concatenation, only use [params] to log arbitrary text that could contain `%` characters.
*
* Only actually does something when logging is enabled.
*
@ -61,7 +65,7 @@ object Log {
}
/**
* Logs the given [throwable] by appending it to [msg]
* Logs the given [throwable] by appending it to [msg].
*
* Only actually does something when logging is enabled.
*
@ -72,7 +76,7 @@ object Log {
debug(getTag(), msg, throwable)
}
/**
* Logs the given [throwable] by appending it to [msg]
* Logs the given [throwable] by appending it to [msg].
*
* Only actually does something when logging is enabled.
*/
@ -82,7 +86,9 @@ object Log {
}
/**
* Logs the given error message by [java.lang.String.format]ting them with an optional list of [params].
* Logs the given message by [String.format][java.lang.String.format]ting them with an optional list of [params]. Since this uses
* [format specifiers][java.util.Formatter], make sure your message only contains `%` characters correctly used as [format specifiers][java.util.Formatter]
* and never do manual string concatenation, only use [params] to log arbitrary text that could contain `%` characters.
*
* Always logs, even in release builds.
*
@ -96,7 +102,9 @@ object Log {
/**
* Logs the given error message by [java.lang.String.format]ting them with an optional list of [params].
* Logs the given message by [String.format][java.lang.String.format]ting them with an optional list of [params]. Since this uses
* [format specifiers][java.util.Formatter], make sure your message only contains `%` characters correctly used as [format specifiers][java.util.Formatter]
* and never do manual string concatenation, only use [params] to log arbitrary text that could contain `%` characters.
*
* Always logs, even in release builds.
*
@ -108,7 +116,7 @@ object Log {
}
/**
* Logs the given [throwable] by appending it to [msg]
* Logs the given [throwable] by appending it to [msg].
*
* Always logs, even in release builds.
*
@ -118,7 +126,7 @@ object Log {
error(getTag(), msg, throwable)
}
/**
* Logs the given [throwable] by appending it to [msg]
* Logs the given [throwable] by appending it to [msg].
*
* Always logs, even in release builds.
*/
@ -173,7 +181,8 @@ fun debug(tag: Tag, msg: String, throwable: Throwable) {
}
private fun doLog(logger: (Tag, String, String) -> Unit, tag: Tag, msg: String, vararg params: Any?) {
logger(tag, Thread.currentThread().name, msg.format(*params))
val formattedMessage = if (params.isEmpty()) msg else msg.format(*params)
logger(tag, Thread.currentThread().name, formattedMessage)
}
private fun isTagDisabled(tag: Tag): Boolean {

26
tests/src/com/unciv/testing/BasicTests.kt
View File

@ -68,7 +68,7 @@ class BasicTests {
var allObsoletingUnitsHaveUpgrades = true
for (unit in units) {
if (unit.obsoleteTech != null && unit.upgradesTo == null && unit.name !="Scout" ) {
debug(unit.name + " obsoletes but has no upgrade")
debug("%s obsoletes but has no upgrade", unit.name)
allObsoletingUnitsHaveUpgrades = false
}
}
@ -94,7 +94,7 @@ class BasicTests {
val ruleset = RulesetCache[baseRuleset.fullName]!!
val modCheck = ruleset.checkModLinks()
if (modCheck.isNotOK())
debug(modCheck.getErrorText(true))
debug("%s", modCheck.getErrorText(true))
Assert.assertFalse(modCheck.isNotOK())
}
}
@ -107,7 +107,7 @@ class BasicTests {
for (paramType in entry.value) {
if (paramType == UniqueParameterType.Unknown) {
val badParam = uniqueType.text.getPlaceholderParameters()[entry.index]
debug("${uniqueType.name} param[${entry.index}] type \"$badParam\" is unknown")
debug("%s param[%s] type \"%s\" is unknown", uniqueType.name, entry.index, badParam)
noUnknownParameters = false
}
}
@ -121,7 +121,7 @@ class BasicTests {
var allOK = true
for (uniqueType in UniqueType.values()) {
if (uniqueType.targetTypes.isEmpty()) {
debug("${uniqueType.name} has no targets.")
debug("%s has no targets.", uniqueType.name)
allOK = false
}
}
@ -135,7 +135,7 @@ class BasicTests {
for (unit in units) {
for (unique in unit.uniques) {
if (!UniqueType.values().any { it.placeholderText == unique.getPlaceholderText() }) {
debug("${unit.name}: $unique")
debug("%s: %s", unit.name, unique)
allOK = false
}
}
@ -150,7 +150,7 @@ class BasicTests {
for (building in buildings) {
for (unique in building.uniques) {
if (!UniqueType.values().any { it.placeholderText == unique.getPlaceholderText() }) {
debug("${building.name}: $unique")
debug("%s: %s", building.name, unique)
allOK = false
}
}
@ -165,7 +165,7 @@ class BasicTests {
for (promotion in promotions) {
for (unique in promotion.uniques) {
if (!UniqueType.values().any { it.placeholderText == unique.getPlaceholderText() }) {
debug("${promotion.name}: $unique")
debug("%s: %s", promotion.name, unique)
allOK = false
}
}
@ -183,7 +183,7 @@ class BasicTests {
for (obj in objects) {
for (unique in obj.uniques) {
if (!UniqueType.values().any { it.placeholderText == unique.getPlaceholderText() }) {
debug("${obj.name}: $unique")
debug("%s: %s", obj.name, unique)
allOK = false
}
}
@ -265,16 +265,16 @@ class BasicTests {
if (replacementTextUnique.type == null) {
debug("${uniqueType.name}'s deprecation text \"$uniqueText\" does not match any existing type!")
debug("%s's deprecation text \"%s\" does not match any existing type!", uniqueType.name, uniqueText)
allOK = false
}
if (replacementTextUnique.type == uniqueType) {
debug("${uniqueType.name}'s deprecation text references itself!")
debug("%s's deprecation text references itself!", uniqueType.name)
allOK = false
}
for (conditional in replacementTextUnique.conditionals) {
if (conditional.type == null) {
debug("${uniqueType.name}'s deprecation text contains conditional \"${conditional.text}\" which does not match any existing type!")
debug("%s's deprecation text contains conditional \"%s\" which does not match any existing type!", uniqueType.name, conditional.text)
allOK = false
}
}
@ -284,7 +284,7 @@ class BasicTests {
while (replacementUnique.getDeprecationAnnotation() != null) {
if (iteration == 10) {
allOK = false
debug("${uniqueType.name}'s deprecation text never references an undeprecated unique!")
debug("%s's deprecation text never references an undeprecated unique!", uniqueType.name)
break
}
iteration++
@ -302,7 +302,7 @@ class BasicTests {
Thread.sleep(5000) // makes timings a little more repeatable
val startTime = System.nanoTime()
statMathRunner(iterations = 1_000_000)
debug("statMathStressTest took ${(System.nanoTime()-startTime)/1000}µs")
debug("statMathStressTest took %sµs", (System.nanoTime()-startTime) / 1000)
}
@Test

2
tests/src/com/unciv/testing/SerializationTests.kt
View File

@ -112,7 +112,7 @@ class SerializationTests {
val pattern = """\{(\w+)\${'$'}delegate:\{class:kotlin.SynchronizedLazyImpl,"""
val matches = Regex(pattern).findAll(json)
matches.forEach {
debug("Lazy missing `@delegate:Transient` annotation: " + it.groups[1]!!.value)
debug("Lazy missing `@delegate:Transient` annotation: %s", it.groups[1]!!.value)
}
val result = matches.any()
Assert.assertFalse("This test will only pass when no serializable lazy fields are found", result)

12
tests/src/com/unciv/testing/TranslationTests.kt
View File

@ -96,7 +96,7 @@ class TranslationTests {
for (placeholder in placeholders) {
if (!output.contains(placeholder)) {
allTranslationsHaveCorrectPlaceholders = false
debug("Placeholder `$placeholder` not found in `$language` for entry `$translationEntry`")
debug("Placeholder `%s` not found in `%s` for entry `%s`", placeholder, language, translationEntry)
}
}
}
@ -116,7 +116,7 @@ class TranslationTests {
val keyFromEntry = translationEntry.replace(squareBraceRegex, "[]")
if (key != keyFromEntry) {
allPlaceholderKeysMatchEntry = false
debug("Entry $translationEntry found under bad key $key")
debug("Entry %s found under bad key %s", translationEntry, key)
break
}
}
@ -136,7 +136,7 @@ class TranslationTests {
for (placeholder in placeholders)
if (placeholders.count { it == placeholder } > 1) {
noTwoPlaceholdersAreTheSame = false
debug("Entry $translationEntry has the parameter $placeholder more than once")
debug("Entry %s has the parameter %s more than once", translationEntry, placeholder)
break
}
}
@ -152,7 +152,7 @@ class TranslationTests {
var failed = false
for (line in templateLines) {
if (line.endsWith(" =")) {
debug("$line ends without a space at the end")
debug("%s ends without a space at the end", line)
failed = true
}
}
@ -177,7 +177,7 @@ class TranslationTests {
translationEntry.entry.tr()
} catch (ex: Exception) {
allWordsTranslatedCorrectly = false
debug("Crashed when translating ${translationEntry.entry} to $language")
debug("Crashed when translating %s to %s", translationEntry.entry, language)
}
}
}
@ -198,7 +198,7 @@ class TranslationTests {
|| translation.count { it == '\"' } != 2
) {
allTranslationsCheckedOut = false
debug("Translation of the word boundary in $language was incorrectly formatted")
debug("Translation of the word boundary in %s was incorrectly formatted", language)
}
}