Specify encoding when asserting a snapshot

[daddykotex]

Unfortunately, I'm currently working from a WSL (Windows Subsystem Linux) and the default encoding is not UTF-8. So for example, I'm writing some tests write now to fetch a HTML page and assert it does not change (after a rewrite), unfortunately, the encoding being unspecified, is not UTF-8 and my tests are failing because of weird French accents!

Would you be open to a PR to have a way to specify the encoding when writing the snapshots? I think the problematic method, at least on the JVM , is fileWriteContents.

I had similar issues in other places in my project, and I'd probably fix that by re-writing with something like:

import java.nio.charset.StandardCharsets
import java.nio.file.Files
import java.nio.file.Paths

val writer = Files.newBufferedWriter(Paths.get(s), StandardCharsets.UTF_8)

What do you think?

I guess this could also be configurable through sbt options? Maybe?

Let me know, and thanks for the lib.

[keynmol]

unfortunately, the encoding being unspecified, is not UTF-8 and my tests are failing because of weird French accents!

Would it work if UTF-8 is enforced on reads and writes?
A PR to enforce that would be fine (existing snapshots would need to be migrated though)
JVM and Native should be fine, JS will likely need an update to the binding: https://nodejs.org/api/fs.html#fswritefilesyncfile-data-options

[daddykotex]

Well, now that you ask, I went and tried to repro and I'm surprised by what I see. I get the write content written to disk when I specify the encoding and the wrong thing when I don't which is what I expected. I can confirm that by catting the file to the console.

But when I read the file from the scala I have the wrong characters.

Take this scala-cli script, that you can run with:

code

import java.io.File
import java.io.FileWriter
import java.nio.file.StandardOpenOption
import java.nio.file.OpenOption
import java.nio.file.{Paths, Files}
import java.nio.charset.{Charset, StandardCharsets}

def write(file: String, s: String, encoding: Option[Charset]): Unit = {
    val writer = encoding match {
        case Some(value) =>
            new FileWriter(new File(file))
        case None =>
            Files.newBufferedWriter(Paths.get(file), StandardOpenOption.TRUNCATE_EXISTING)
    }
    try {
        writer.write(s)
    } finally {
        writer.close()
    }
}

def read(file: String, encoding: String): Unit = {
    val source = scala.io.Source.fromFile(file, encoding)
    val content = source.mkString
    println(content)
}


val content = "Écoute capitaine!"

// specify encoding
val file = "test.txt"
write(file, content, Some(StandardCharsets.UTF_8))
read(file, StandardCharsets.UTF_8.name())


// unspecified through FileWriter
val file2 = "test2.txt"
write(file2, content, None)
read(file2,  StandardCharsets.UTF_8.name())

scala-cli run --scala-version 2.13 read-write.sc
Compiling project (Scala 2.13.18, JVM (11))
Compiled project (Scala 2.13.18, JVM (11))
?coute capitaine!
?coute capitaine!
> cat test.txt
?coute capitaine!¶                                                                                                                                                                                                                    
> cat test2.txt
Écoute capitaine!¶

So, I think specifying is the way to go to ensure consistent behavior across platform, but I'm not sure why I don't read the content correctly from Scala (my other app deals with it just right).

If it is an optional option, it would be an opt-in, so no migration would be needed, correct?

[keynmol]

so no migration would be needed

Yeah I mean you'd have to re-encode your own snapshots, so migration only for you :)

I think the way to go about it is to:

1. Add an extra overload of https://github.com/indoorvivants/snapshot-testing/blob/main/modules/snapshots-buildtime/src/main/resources/MunitSnapshotsIntegration.scala#L53 which takes encoding
2. Make Snapshots runtime take encoding as a constructor parameter, defaulting ot UTF8
3. Change all methods in Snapshots to correctly pass the encoding to filesystem methods
4. The overload in (1) can just copy existing Snapshots instance (which is generated by the build tool) in case an encoding change is needed

But I think that forcing UTF-8 everywhere would be a sufficient first step.

Your repro works fine for me on MacOS, but it probably defaults to UTF8

[daddykotex]

Looked at this last night, I think I've got it to work

Would you also like to see a build settings to configure the default encoding (like you have one for overwriting: snapshotsForceOverwrite)?

[keynmol]

Don't think we need a build setting.

I think at this point it makes sense only to allow varying the encoding on per-snapshot level, and globally defaulting to the utf-8. Users who need to change encoding globally can just add an overload to assertSnapshot with desired encoding set.

snapshotsForceOverwrite is a bit different because it changes the logic of assertions and is designed to handle CI/local dev differently.