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

Support multiple snapshots for one test, with custom filename disambiguation #70

Open
Utsira opened this issue Apr 21, 2020 · 6 comments

Comments

@Utsira
Copy link

Utsira commented Apr 21, 2020

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)")
			}
		}
@KaneCheshire
Copy link
Owner

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

@KaneCheshire
Copy link
Owner

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?

@Utsira
Copy link
Author

Utsira commented Apr 21, 2020

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

@KaneCheshire
Copy link
Owner

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

@Utsira
Copy link
Author

Utsira commented Apr 21, 2020

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
Copy link
Author

Utsira commented Apr 21, 2020

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)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants