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

Feature Request: Provide a way of simplifying snapshot name #549

Open
Chrisjc28 opened this issue Aug 18, 2022 · 8 comments · May be fixed by #1292
Open

Feature Request: Provide a way of simplifying snapshot name #549

Chrisjc28 opened this issue Aug 18, 2022 · 8 comments · May be fixed by #1292
Milestone

Comments

@Chrisjc28
Copy link

Description
It would be great if the snapshot didn't have to have all of the values in it such as the package name, class name and the method name and you could provide a custom name with only the details you wanted.

@JulianEggers
Copy link

I would suggest making the naming schema configurable.
The package of our setup is always the same which makes it unnecessary clutter in the filename.

@jrodbx
Copy link
Collaborator

jrodbx commented Sep 5, 2022

@Chrisjc28 (or anyone) mind elaborating on the "why"? Would help prioritizing if some use cases were provided.

@Chrisjc28
Copy link
Author

We had a recent issue where the memory reference was being used in the name of the snapshot, when running locally all of our tests worked fine. However when running in our CI the test would use a different memory reference in the name causing them to fail. This is a niche issue but if we were able to have more control over what the file name was this wouldn't have happened. We have put something in place as a temporary fix but it has place a code smell in our project i would rather not have.

@jrodbx
Copy link
Collaborator

jrodbx commented Sep 9, 2022

I've seen this before with usages of TestParameterInjector with custom provided values. In those cases, providing a toString method or implementing as a data class is the suggested fix.

@Chrisjc28
Copy link
Author

Chrisjc28 commented Sep 11, 2022

In our case we have used a secondary constructor and it's being changed to a data class, however there is no benefit for us to have the data class.

An example of the class we are using in our test is -

sealed Class AppState {
        object: Loading,
        data class Success(val data: Any)
        data class Error(val error: Any)
} 

For us converting loading to a data class has no benefit and makes us have to make use of a secondary constructor just so we don't have to and parentheses to every instance of loading.

The suggestion of using toString would you mean to implement that in the loading object itself?

@jrodbx jrodbx added this to the 1.3 milestone Dec 15, 2022
@jrodbx jrodbx modified the milestones: 1.3, Backlog Jan 25, 2023
@apkelly
Copy link

apkelly commented Jul 10, 2023

Being able to specify a custom filename would be great, instead of the hard-coded list which includes things like packagename

internal fun Snapshot.toFileName(
  delimiter: String = "_",
  extension: String
): String {
  val formattedLabel = if (name != null) {
    "$delimiter${name.toLowerCase(Locale.US).replace("\\s".toRegex(), delimiter)}"
  } else {
    ""
  }
  return "${testName.packageName}${delimiter}${testName.className}${delimiter}${testName.methodName}$formattedLabel.$extension"
}

Being able to provide a custom filename would allow me to remove the packageName part, which is the same for all my tests e.g. com.example.tests.ui.paparazzi, I only really need className and methodName.

@KatieBarnett
Copy link

I would like this, the excessive name also makes it truncate in github and makes it hard to find files in finder.

@jrodbx jrodbx modified the milestones: Backlog, 1.4 Jan 19, 2024
magnusvs added a commit to magnusvs/paparazzi that referenced this issue Feb 14, 2024
Adds a FileNameProvider that can be implemented in any way to specify file names for a recorded Snapshot.
Closes feature request cashapp#549
@magnusvs magnusvs linked a pull request Feb 14, 2024 that will close this issue
magnusvs added a commit to magnusvs/paparazzi that referenced this issue Feb 14, 2024
Adds a FileNameProvider that can be implemented in any way to specify file names for a recorded Snapshot.
Closes feature request cashapp#549
magnusvs added a commit to magnusvs/paparazzi that referenced this issue Feb 14, 2024
Adds a FileNameProvider that can be implemented in any way to specify file names for a recorded Snapshot.
Closes feature request cashapp#549
@micHar
Copy link

micHar commented Nov 17, 2024

There's a nice PR for this feature and it seems like it's widely requested. Any reasons not to merge it?

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

Successfully merging a pull request may close this issue.

7 participants