Skip to content
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.

Sign up for GitHub

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

Tweak Maven build gen tests #4079

Merged
merged 8 commits into from
Dec 11, 2024
Merged

Tweak Maven build gen tests #4079

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ab1c45d
Tweak Maven build gen tests
alexarchambault Dec 5, 2024
5b8eccc
Use 'git diff --no-index' to print diff between expected / actual
alexarchambault Dec 6, 2024
5a0b32f
Use git --diff / os.copy to check and update test data
alexarchambault Dec 10, 2024
e21e71e
Clean-up
alexarchambault Dec 10, 2024
a339e3c
Try not to change committed test data perms upon checkout
alexarchambault Dec 10, 2024
79672c1
Revert "Try not to change committed test data perms upon checkout"
alexarchambault Dec 10, 2024
e460670
Other attempt
alexarchambault Dec 10, 2024
cbd729d
fixup
alexarchambault Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 38 additions & 9 deletions main/maven/test/src/mill/main/maven/BuildGenTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@ package mill.main.maven

import mill.T
import mill.api.PathRef
import mill.main.client.OutFiles
import mill.scalalib.scalafmt.ScalafmtModule
import mill.testkit.{TestBaseModule, UnitTester}
import utest.*
import utest.framework.TestPath

import java.nio.file.FileSystems

object BuildGenTests extends TestSuite {

// Change this to true to update test data on disk
def updateSnapshots = false

def tests: Tests = Tests {
val resources = os.Path(sys.env("MILL_TEST_RESOURCE_DIR"))
val scalafmtConfigFile = PathRef(resources / ".scalafmt.conf")
Expand All @@ -33,7 +39,7 @@ object BuildGenTests extends TestSuite {
eval(module.reformat())

// test
checkFiles(files, resources / expectedRel)
checkFiles(files.map(_.path.relativeTo(dest).asSubPath), dest, resources / expectedRel)
}

// multi level nested modules
Expand Down Expand Up @@ -125,15 +131,38 @@ object BuildGenTests extends TestSuite {
.map(PathRef(_))
.toSeq

def checkFiles(actualFiles: Seq[PathRef], expectedRoot: os.Path): Boolean = {
val expectedFiles = buildFiles(expectedRoot)
def checkFiles(actualFiles: Seq[os.SubPath], root: os.Path, expectedRoot: os.Path): Boolean = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use git diff --no-index to perform this comparison? That comes with a nice diff viewer for free for people to understand what the difference between the two folders is, and means a lot less comparison code for us to maintain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying that. That output is mainly meant for CI IMO. Locally, one can set updateSnapshots to true, and run git diff manually to check the diff.

// Non *.mill files, that are not in test data, that we don't want
// to see in the diff
val toCleanUp = os.walk(root, skip = _.startsWith(root / OutFiles.defaultOut))
.filter(os.isFile)
.filter(!_.lastOpt.exists(_.endsWith(".mill")))
toCleanUp.foreach(os.remove)

// Try to normalize permissions while not touching those of committed test data
val supportsPerms = FileSystems.getDefault.supportedFileAttributeViews().contains("posix")
if (supportsPerms)
for {
testFile <- os.walk(expectedRoot)
if os.isFile(testFile)
targetFile = root / testFile.relativeTo(expectedRoot).asSubPath
if os.isFile(targetFile)
}
os.perms.set(targetFile, os.perms(testFile))

val diffExitCode = os.proc("git", "diff", "--no-index", expectedRoot, root)
.call(stdin = os.Inherit, stdout = os.Inherit, check = !updateSnapshots)
.exitCode

actualFiles.nonEmpty &&
actualFiles.length == expectedFiles.length &&
actualFiles.iterator.zip(expectedFiles.iterator).forall {
case (actual, expected) =>
actual.path.endsWith(expected.path.relativeTo(expectedRoot)) &&
os.read.lines(actual.path) == os.read.lines(expected.path)
if (updateSnapshots && diffExitCode != 0) {
System.err.println(
s"Expected and actual files differ, updating expected files in resources under $expectedRoot"
)

os.remove.all(expectedRoot)
os.copy(root, expectedRoot)
}

diffExitCode == 0 || updateSnapshots
}
}
Loading