PixelTest icon indicating copy to clipboard operation
PixelTest copied to clipboard

Support multiple snapshots for one test, with custom filename disambiguation

Open Utsira opened this issue 4 years ago • 6 comments

Usecase:

Sometimes you want to have multiple snapshots in one test function. Currently this isn't possible as PixelTest only disambiguates the files by filename, function name, and image size. For instance, say you have a view that is configured by 2 enums Kind and Status. Say Kind has 5 cases, and Status has 3 cases. Both conform to CaseIterable. You want to test all possible configurations (currently 15) by iterating over all cases of both enums. This has the advantage of a. reducing boilerplate test code, and b. automatically adding new snapshots if, eg, a sixth case gets added to Kind.

For this to work, PixelTest would need a way to differentiate different snapshots coming from the same test function. I believe the nicest way to do this, would be if you could supply PixelTest with an optional "filenameSuffix" argument. For example, if the Kind and Status enums both had a developer-friendly description implementation, the call site might look like this:

		for kind in Kind.allCases {
			for status in Status.allCases {
				cell.update(with: kind, status: status)
				verify(cell, layoutStyle: .dynamicHeight, filenameSuffix: "\(kind)_\(status)")
			}
		}

Utsira avatar Apr 21 '20 11:04 Utsira

Not exactly the same but maybe related to #50 as well.

KaneCheshire avatar Apr 21 '20 11:04 KaneCheshire

I was thinking about this the other day when I tried to do the same thing, so definitely keen for this to be in PixelTest, however I would much prefer it if it was something that PixelTest could figure out itself somehow, with maybe an option for devs to override it if required (although I'd rather leave that until we know it's definitely something that is needed to be added).

Initial thoughts are maybe some sort of counter for the amount of times verify is called, which is used in the file name?

KaneCheshire avatar Apr 21 '20 11:04 KaneCheshire

Sure, PixelTest could have a default disambiguator. But it would make the snapshots less useful if they were named allCardTypes_1, allCardTypes_2 etc. It's a lot easier to say "this is what card X looks like in Y state" if the files were named eg allCardTypes_blocked_visa, allCardTypes_active_visa etc

Utsira avatar Apr 21 '20 11:04 Utsira

Good point, how about some sort of context function then? A bit like the BDD scenarios? I just think it might look a bit ugly/hard to read if it's just a long string in the existing verify function. Instead something maybe like:

func test_myView() {
  context("state a") {
    verify(view, layoutStyle:...)
  }
}

Btw this just gave me an idea and it's actually already possible to have support for this without needing any changes to PixelTest., using nested functions:

func test_myView() {
  func myView_stateA() {
    verify(view, layoutStyle:...)
  }
  func myView_stateB() {
   verify(view, layoutStyle:...)
  }
  stateA()
  stateB()
}

Since PixelTest uses the function name that calls verify, it will use each nested function name.

This is obviously gross and we should find a better way but it's definitely possible for now until there's a better way

KaneCheshire avatar Apr 21 '20 11:04 KaneCheshire

I like your suggestion of the context function. Nested functions wouldn't help with the use case I described though, because function names are always going to be static strings, by definition you can't generate them programmatically in a for loop

Utsira avatar Apr 21 '20 11:04 Utsira

Though I'm not sure I buy the argument about the verify command being ugly if it has an extra argument. If you need to wrap verify in a helper function, then you end up having to pass in file, function, and line values anyway. Without this feature, the way I would do this would be something like:

private func verifyCellForCard(kind: Kind, status: Status, file: StaticString = #file, function: StaticString = #function, line: UInt = #line) {
// setup the card
verify(cell, layoutStyle: .dynamicHeight, file: file, function: function, line: line)
}

Utsira avatar Apr 21 '20 11:04 Utsira