Skip to content

Commit

Permalink
Merge pull request #2684 from pinterest/regression-test-before-releas…
Browse files Browse the repository at this point in the history
…e-1.3.0

Regression test before release 1.3.0
  • Loading branch information
paul-dingemans authored Jun 4, 2024
2 parents fc164be + 628cfc7 commit 6e944d7
Show file tree
Hide file tree
Showing 30 changed files with 293 additions and 236 deletions.
66 changes: 36 additions & 30 deletions RELEASE_TESTING.MD
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Before releasing a new version of KtLint, the release candidate is tested on a s
```
4. Create an alias or script to run the latest released version of ktlint (note that this script will print the version as reference to which version is used):
```shell
alias ktlint-prev="ktlint-1.1.0 $@" # Replace with the latest release version
alias ktlint-prev="ktlint-1.2.1 $@" # Replace with the latest release version
```
Note that `~/git/ktlint` is the directory in which the ktlint project is checked out and that `~/git/ktlint/ktlint` refers to the `ktlint` CLI-module.
5. Create an alias or script to run the latest development-version of ktlint (note that this script will print the version and the datetime of compilation as reference to which version is used):
Expand Down Expand Up @@ -84,52 +84,57 @@ Formatting projects in which ktlint is not used may result in a huge amount of f
# be changed:
# 1) Disable the "root = true" property so that each project ultimately falls back on this file. In this way offending
# rules can be easily enabled/disabled for all test projects
# 2) Add specific rules to project below
# graphql
# 2) Remove property below from graphql-kotlin
# disabled_rules=import-ordering
# And add properties below:
# ktlint_standard_import-ordering = disabled
# ktlint_standard_package-name = disabled
ktlint_code_style = ktlint_official
ktlint_experimental = enabled
ktlint_standard = enabled
ktlint_standard_filename = disabled
ktlint_standard_no-wildcard-imports = disabled
ktlint_standard_function-naming = disabled
ktlint_standard_property-naming = disabled
ktlint_standard_class-naming = disabled
ktlint_standard_comment-wrapping = disabled
```
2. Commit changes:
```shell
./exec-in-each-project.sh "git add --all && git commit -m \"Update .editorconfig to fallback to integration test settings\""
```
3. Format the sample projects with the *latest released* ktlint version:
3. Build baseline file with previous (*latest released*) version of Lint (when building with format the offsets of the error are not saved correctly in the baseline)
```shell
rm baseline.xml
ktlint-prev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script.
```
Note: Ignore all output as this is the old version!
4. Format the sample projects with the previous (*latest released*) ktlint version:
```shell
ktlint-prev -F --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script.
```
Note: Ignore all output as this is the old version!
4. Commit changes:
5. Commit changes:
```shell
./exec-in-each-project.sh "git add --all && git commit -m \"Format with previous ktlint version -F\""
./exec-in-each-project.sh "git add --all && git commit -m \"Format with previous ktlint version (round #)\""
```
Repeat step 3 and 4 until no files are changed anymore. Although ktlint reruns up to 3 times in case new violations are introduced, it can still happen that not all violations have been fixed with a single invocation.
5. Check that besides the `baseline.xml` no files are changed (in step 3 and 4 all violations which could be autocorrected have already been committed). Remaining violations which could not be autocorrected are saved in the `baseline.xml` which is stored outside the project directories.
Repeat step 4 and 5 until no files are changed anymore. Although ktlint reruns up to 3 times in case new violations are introduced, it can still happen that not all violations have been fixed with a single invocation.
6. Check that besides the `baseline.xml` no files are changed (in step 4 and 5 all violations which could be autocorrected have already been committed). Remaining violations which could not be autocorrected are saved in the `baseline.xml` which is stored outside the project directories.
```shell
./exec-in-each-project.sh "git status"
```
The `baseline.xml` file should only contain errors which can not be autocorrected.
6. Lint with *latest development* version:
7. Rebuild baseline file with previous (*latest released*) ktlint (when building with format the offsets of the error are not saved correctly in the baseline) so that all violations are now stored in the baseline. Note the old `baseline.xml` still contains all errors which are already autocorrected. After running next command, only the error which can not be autocorrected with the previous ktlint version remain.
```shell
ktlint-dev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
rm baseline.xml
ktlint-prev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script.
```
or when lots are violations are reported:
Note: Ignore all output as this is the old version!
8. Lint with *latest development* version:
```shell
ktlint-dev --baseline=baseline.xml --relative --reporter=plain-summary # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
ktlint-dev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
```
Inspect the output roughly (detailed inspection is done when formatting):
* Is the amount of logging messages comparable to before? If not, are the changes intended?
* Are violations related to rules that have actually been added or changed?
7. Format with *latest development* version:
* If you see an error like below, then this version obviously may *not* be released. It is best to fix this error before continuing with testing and validating!
```plain
Internal Error (...) in file '...' at position '0:0. Please create a ticket at https://github.com/pinterest/ktlint/issues ...
```
9. Format with *latest development* version:
```shell
ktlint-dev -F --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
```
Expand All @@ -138,20 +143,21 @@ Formatting projects in which ktlint is not used may result in a huge amount of f
```plain
Internal Error (...) in file '...' at position '0:0. Please create a ticket at https://github.com/pinterest/ktlint/issues ...
```
* Ideally, no violations are shown. This means that all violations have been autocorrected.
* Usually it helps to disable all rules that emit violations, except one of those rules. In this way it is possible to evaluate the changes rule by rule.
* Ideally, no violations are shown. This means that all violations have been autocorrected. Note that violations might pop up that previously were suppressed via the baseline. This can happen as due to code changes, the references in the baseline.xml no longer match with the positions where they occur. First check the code changes, and regenerating the baseline before verifying the next can be a helpful approach.
* Violations which could not be autocorrected should be validated for correctness but do not block the release as most likely this is intended behavior.
* If a violation is shown which is not marked as being "can not be autocorrected" this means that during autocorrect of another violation a new violations has been introduced. This should be fixed before releasing especially when the next format introduces the original violation again which of course would result in an endless loop.
8. Inspect all fixed violations, Of course inspection similar violations tens of times does not make sense. At least check different types of violations a couple of times. Commit changes which do not need to be inspected again:
```shell
./exec-in-each-project.sh "git add --all && git commit -m \"Fixed with latest development version\""
```
9. Rerun lint with *latest development* version and create a new baseline:
10. Inspect all fixed violations, Of course inspection similar violations tens of times does not make sense. At least check different types of violations a couple of times. Commit changes which do not need to be inspected again:
```shell
rm baseline.xml
ktlint-dev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
./exec-in-each-project.sh "git add --all && git commit -m \"Fixed with latest development version\""
```
No violations, except error that can not be autocorrected, should be expected.
10. Rerun with *latest development* version and verify that no violations are reported:
11. Rerun lint with *latest development* version and create a new baseline:
```shell
rm baseline.xml
ktlint-dev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
```
No violations, except error that can not be autocorrected, should be expected.
12. Rerun with *latest development* version and verify that no violations are reported:
```shell
ktlint-dev --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want one combined baseline.xml file for all projects.
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ internal class CodeFormatter(
}
formatRunCount++
} while (mutated && formatRunCount < maxFormatRunsPerFile)
if (mutated && formatRunCount == maxFormatRunsPerFile) {
if (mutated && formatRunCount == maxFormatRunsPerFile && autocorrectHandler !is NoneAutocorrectHandler) {
// It is unknown if the last format run introduces new lint violations which can be autocorrected. So run lint once more
// so that the user can be informed about this correctly.
lintAfterFormat().also {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.pinterest.ktlint.rule.engine.core.api.nextCodeSibling
import com.pinterest.ktlint.rule.engine.core.api.nextLeaf
import com.pinterest.ktlint.rule.engine.core.api.nextSibling
import com.pinterest.ktlint.rule.engine.core.api.prevSibling
import com.pinterest.ktlint.rule.engine.core.api.remove
import com.pinterest.ktlint.ruleset.standard.StandardRule
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
Expand Down Expand Up @@ -97,11 +98,11 @@ public class AnnotationSpacingRule : StandardRule("annotation-spacing") {
// Remove the annotation and the following whitespace
val eolComment = node.nextSibling { it.isCommentOnSameLineAsPrevLeaf() }
if (eolComment != null) {
eolComment.prevSibling { it.isWhiteSpace() }?.let { it.treeParent.removeChild(it) }
eolComment.nextSibling { it.isWhiteSpace() }?.let { it.treeParent.removeChild(it) }
eolComment.prevSibling { it.isWhiteSpace() }?.remove()
eolComment.nextSibling { it.isWhiteSpace() }?.remove()
eolComment.treeParent?.removeChild(eolComment)
} else {
node.nextSibling { it.isWhiteSpace() }?.let { it.treeParent?.removeChild(it) }
node.nextSibling { it.isWhiteSpace() }?.remove()
}
node.treeParent.removeChild(node)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import com.pinterest.ktlint.rule.engine.core.api.nextSibling
import com.pinterest.ktlint.rule.engine.core.api.prevCodeSibling
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.rule.engine.core.api.prevSibling
import com.pinterest.ktlint.rule.engine.core.api.remove
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
import com.pinterest.ktlint.ruleset.standard.StandardRule
import org.ec4j.core.model.PropertyType
Expand Down Expand Up @@ -368,9 +369,7 @@ public class ChainMethodContinuationRule :
.takeIf { it.isWhiteSpaceWithNewline() }
?.let { whiteSpace ->
emit(whiteSpace.startOffset - 1, "Unexpected newline after '${chainOperator.text}'", true)
.ifAutocorrectAllowed {
whiteSpace.treeParent.removeChild(whiteSpace)
}
.ifAutocorrectAllowed { whiteSpace.remove() }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import com.pinterest.ktlint.rule.engine.core.api.nextSibling
import com.pinterest.ktlint.rule.engine.core.api.prevCodeLeaf
import com.pinterest.ktlint.rule.engine.core.api.prevCodeSibling
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.rule.engine.core.api.remove
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
import com.pinterest.ktlint.ruleset.standard.StandardRule
Expand Down Expand Up @@ -96,7 +97,7 @@ public class ChainWrappingRule :
node.upsertWhitespaceBeforeMe(indentConfig.childIndentOf(node))
node.upsertWhitespaceAfterMe(" ")
} else {
node.treeParent.removeChild(node)
node.remove()
(nextLeaf as LeafElement).rawInsertAfterMe(node as LeafElement)
}
}
Expand Down Expand Up @@ -150,12 +151,12 @@ public class ChainWrappingRule :
operationReference
.prevCodeSibling()
?.nextSibling()
operationReference.treeParent.removeChild(operationReference)
operationReference.remove()
insertBeforeSibling?.treeParent?.addChild(operationReference, insertBeforeSibling)
node.treeParent.upsertWhitespaceBeforeMe(" ")
} else {
val insertionPoint = prevLeaf.prevCodeLeaf() as LeafPsiElement
(node as LeafPsiElement).treeParent.removeChild(node)
(node as LeafPsiElement).remove()
insertionPoint.rawInsertAfterMe(node)
(insertionPoint as ASTNode).upsertWhitespaceAfterMe(" ")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import com.pinterest.ktlint.rule.engine.core.api.nextLeaf
import com.pinterest.ktlint.rule.engine.core.api.prevCodeSibling
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.rule.engine.core.api.prevSibling
import com.pinterest.ktlint.rule.engine.core.api.remove
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
import com.pinterest.ktlint.ruleset.standard.StandardRule
Expand Down Expand Up @@ -268,9 +269,7 @@ public class ClassSignatureRule :
}?.let { parameterList ->
if (!dryRun) {
emit(parameterList.startOffset, "No parenthesis expected", true)
.ifAutocorrectAllowed {
parameterList.treeParent.removeChild(parameterList)
}
.ifAutocorrectAllowed { parameterList.remove() }
} else {
whiteSpaceCorrection -= parameterList.textLength
}
Expand Down Expand Up @@ -323,9 +322,7 @@ public class ClassSignatureRule :
firstParameter!!.startOffset,
"No whitespace expected between opening parenthesis and first parameter name",
true,
).ifAutocorrectAllowed {
whiteSpaceBeforeIdentifier.treeParent.removeChild(whiteSpaceBeforeIdentifier)
}
).ifAutocorrectAllowed { whiteSpaceBeforeIdentifier.remove() }
} else {
whiteSpaceCorrection -= whiteSpaceBeforeIdentifier.textLength
}
Expand Down Expand Up @@ -434,9 +431,7 @@ public class ClassSignatureRule :
whiteSpaceBeforeClosingParenthesis.startOffset,
"No whitespace expected between last parameter and closing parenthesis",
true,
).ifAutocorrectAllowed {
whiteSpaceBeforeClosingParenthesis.treeParent.removeChild(whiteSpaceBeforeClosingParenthesis)
}
).ifAutocorrectAllowed { whiteSpaceBeforeClosingParenthesis.remove() }
} else {
whiteSpaceCorrection -= whiteSpaceBeforeClosingParenthesis.textLength
}
Expand Down Expand Up @@ -582,9 +577,7 @@ public class ClassSignatureRule :
.findChildByType(WHITE_SPACE)
?.let { whitespace ->
emit(whitespace.startOffset, "No whitespace expected", true)
.ifAutocorrectAllowed {
whitespace.treeParent.removeChild(whitespace)
}
.ifAutocorrectAllowed { whitespace.remove() }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INSERT_FINAL_NEWLINE_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.ifAutocorrectAllowed
import com.pinterest.ktlint.rule.engine.core.api.isRoot
import com.pinterest.ktlint.rule.engine.core.api.remove
import com.pinterest.ktlint.ruleset.standard.StandardRule
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
Expand Down Expand Up @@ -45,9 +46,7 @@ public class FinalNewlineRule :
} else {
if (lastNode is PsiWhiteSpace && lastNode.textContains('\n')) {
emit(lastNode.startOffset, "Redundant newline (\\n) at the end of file", true)
.ifAutocorrectAllowed {
lastNode.node.treeParent.removeChild(lastNode.node)
}
.ifAutocorrectAllowed { lastNode.node.remove() }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.pinterest.ktlint.rule.engine.core.api.parent
import com.pinterest.ktlint.rule.engine.core.api.prevCodeSibling
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.rule.engine.core.api.prevSibling
import com.pinterest.ktlint.rule.engine.core.api.remove
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
import com.pinterest.ktlint.ruleset.standard.StandardRule
Expand Down Expand Up @@ -372,8 +373,8 @@ public class FunctionLiteralRule :
arrow
.nextSibling()
.takeIf { it.isWhiteSpace() }
?.let { it.treeParent.removeChild(it) }
arrow.treeParent.removeChild(arrow)
?.remove()
arrow.remove()
}
}
}
Expand Down
Loading

0 comments on commit 6e944d7

Please sign in to comment.