Barebones of int selfie

[jknack]

• Attempt to implement int selfie

@nedtwigg I'm a bit lost, not sure if this is what are we looking for.

[nedtwigg]

Sorry for slow response, I'm trying to structure this task a little better. Thanks for taking a shot, I'll have more background ASAP.

[nedtwigg]

@jknack I fleshed this out some more, I'd love your help if you're available! This is the integration test for the user experience that we're working towards.

selfie/selfie-runner-junit5/src/test/kotlin/com/diffplug/selfie/junit5/InlineIntTest.kt

Lines 29 to 40 in 465a08d

	@Test @Order(1)
	fun toBe_TODO() {
	ut_mirror().lineWith("expectSelfie").setContent(" expectSelfie(1234).toBe_TODO()")
	gradleReadSSFail()
	}
	@Test @Order(2)
	fun toBe_write() {
	gradleWriteSS()
	ut_mirror().lineWith("expectSelfie").content() shouldBe " expectSelfie(1234).toBe(1234)"
	gradleReadSS()
	}

The unimplemented code is this part:

selfie/selfie-runner-junit5/src/main/kotlin/com/diffplug/selfie/junit5/WriteTracker.kt

Lines 88 to 112 in 002129d

	// If I was implementing this, I would use Slice https://github.com/diffplug/selfie/pull/22
	// as the type of source, but that is by no means a requirement
	for (location in locations) {
	if (location.first.subpath != subpath) {
	path?.let { Files.writeString(it, source) }
	subpath = location.first.subpath
	deltaLineNumbers = 0
	path = layout.testSourceFile(location.first)
	source = Files.readString(path)
	}
	// parse the location within the file
	val currentlyInFile = "TODO parse using ${location.first.line + deltaLineNumbers}"
	val literalValue = location.second.snapshot
	val parsedInFile = literalValue.format.parse(currentlyInFile)
	if (parsedInFile != literalValue.expected) {
	// warn that the parsing wasn't as expected
	// TODO: we can't report failures to the user very well
	// someday, we should verify that the parse works in the `record()` and
	// throw an `AssertionFail` there so that the user sees it early
	}
	val toInjectIntoFile = literalValue.encodedActual()
	deltaLineNumbers += (toInjectIntoFile.countNewlines() - currentlyInFile.countNewlines())
	// TODO: inject the encoded thing into the file
	}
	path?.let { Files.writeString(it, source) }

The first thing I would do is refactor the code above to make it easier to do fast "unit" test instead of only big slow integration test. Something like this

https://github.com/diffplug/selfie/blob/feat/int-selfie/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/IntFormatTest.kt

[jknack]

sure I'll take it. Thank you @nedtwigg

[nedtwigg]

Great progress, definitely on the right track! My main feedback would be to add a way to test the replacement logic that doesn't need the Harness and UT_ stuff, something like "when testfile is , replacement value is , and callstack is , the result is ".

For now, we can assume test files are only Java / Kotlin. But someday we will want this to support Javascript, Scala, Groovy, Clojure, etc. Those can all be enduser PR's in the future, but we'll want to have good infrastructure to support this parse/replace operation.

We also might want to support wrapper libraries, where someone does Assert.assertEquals(actual, expected) but it's just a wrapper for selfie, which will replace the value in the assertEquals() line. We shoudn't complicate our code for this now, we should do the simplest thing that works, so I think your regex is perfect.

But over time, as we get enduser PRs to support more usecases, this code will see a lot of traffic. So spare no expense when it comes to framing out its internal API and testing.

[nedtwigg]

I don't see why it needs anything more than line.conains(start). The trick is to do linesFrom only for things where you know the line is unique. If this was enduser API I would probably enforce that it must be unique, and throw an error with line numbers if more than one line matched. If we're having trouble with it in our test code, then I'd support adding a uniqueness check here.

[jknack]

Hi @nedtwigg,

Sorry for being lost... I'm back if you still want to work with me. Thanks

[nedtwigg]

Hi @jknack, thrilled to have you back! No worries, I would still love your help on this.

The inline snapshot stuff is exactly where you left it.

My comment above is imo still the relevant next step. If it's confusing, just let me know and I can take a try at writing the harness myself to demonstrate what I mean.

[nedtwigg]

Just FYI @jknack, I've been integrating Selfie into our website, and have a bunch of changes I've made to make the integration work.

I've also decided to rename the packages from com.diffplug.selfie to dev.selfie. I'm going to:

• break up all my changes into focused PRs
• get this PR passing and merge it into main
• rename com.diffplug.selfie to dev.selfie
• improve the test harnessing for inline snapshots

Once I'm at that point I would love your help improving the inline snapshot mechanism if you're available. I'm hoping to get this all done by tomorrow evening, TBD.

[nedtwigg]

Hi @jknack. I've decided not to rename packages for now. There's a ton of open work in

• Inline snapshot roadmap #48

If you have time, just go down the list and implement. I'm gonna implement one more thing from the list then it's all yours.

[jknack]

@nedtwigg thank you