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.

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

Remove requirement to not have "/" in test names? (Junit 5) #514

Open
veeti opened this issue Jan 24, 2025 · 1 comment
Open

Remove requirement to not have "/" in test names? (Junit 5) #514

veeti opened this issue Jan 24, 2025 · 1 comment
Labels
enhancement New feature or request jvm

Comments

@veeti
Copy link

veeti commented Jan 24, 2025

Fantastic tool, thank you! I only hit a little snag: in my project I already have many Kotest/Junit5 units named after HTTP endpoints such as GET /example, and I'm hitting this error:

Caused by: java.lang.IllegalStateException: Test name cannot contain '/', was ...
	at com.diffplug.selfie.junit5.SnapshotFileProgress.startTest(SnapshotSystemJUnit5.kt:188)

This happens for every test even though they do not invoke Selfie for snapshots. Could this requirement be loosened somehow? For example:

  • Escape the resulting name of snapshot files to remove the slash
  • Do not run this check in tests that do not make use of Selfie

Thank you.

@nedtwigg nedtwigg added bug Something isn't working jvm enhancement New feature or request and removed bug Something isn't working labels Jan 24, 2025
@nedtwigg
Copy link
Member

Workarounds:

  • in your test names, replace / with some other filename-safe character like % or even division slash (if you're crazy).
  • have two test suites (Gradle makes this pretty easy, not sure about Maven), and only add Selfie to one of them.

I would love to merge a PR which relaxes this requirement, I think you nailed the two paths forward:

Do not run this check in tests that do not make use of Selfie

I think this will be hard. An intentional design decision in Selfie was "add the dependency and it just works". A downside of this is that the Selfie test runner can't know ahead of time whether any given test is going to use Selfie or not. Delaying the check to a lazy "only if used" is possible, but looks tricky to me. If it adds a ton of complexity to the code, then I don't want to merge it.

Escape the resulting name of snapshot files to remove the slash

This would be great! Selfie already has a rigorous escape system so that snapshot names and contents can be literally anything without causing any problems:

/**
* https://github.com/diffplug/selfie/blob/main/jvm/selfie-lib/src/commonTest/resources/com/diffplug/selfie/scenarios_and_lenses.ss
*/
internal val nameEsc = PerCharacterEscaper.specifiedEscape("\\\\[(])\nn\tt╔┌╗┐═─")
/** https://github.com/diffplug/selfie/issues/2 */
internal val bodyEsc = PerCharacterEscaper.selfEscape("\uD801\uDF43\uD801\uDF41")

I think the right thing is to have a filenameEsc = PerCharacterEscaper.specifiedEscape("@@<(>):c\"q/f\b|p?q*s"). This would create an escaper that does the following transformation:

  • @ -> @@
  • < -> @(
  • " -> @q
  • / -> @f (forward slash)
  • etc

It should be pretty straightforward from there, but there might be some gotchas I haven't thought of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request jvm
Projects
None yet
Development

No branches or pull requests

2 participants