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

Ability to set screenshot filename formats for screenshots generated by on_failure and on_error - take 2 #2023

Closed
pmorch opened this issue Feb 11, 2019 · 6 comments

Comments

@pmorch
Copy link

pmorch commented Feb 11, 2019

This is a dup of #1620. However #1620 was closed by the stale bot and I can't reopen it. The issue still has merit so I (re-)submit the issue.

For our purposes, we'd just like the file names easily parsable and since the terminal's ls -l sorts by filename by default, make sure the filenames start with the year, month, day, hour etc.

Currently, lib/testsuite/screenshots.js:18 has:

static getScreenshotFileName(currentTest, isError, screenshotsPath) {
  // code that creates prefix and dateStamp from the above params using a hardcoded format
  return path.resolve(path.join(screenshotsPath, `${prefix}_${dateStamp}.png`));
}

And so nothing is configurable. It seems it wouldn't be so hard to refactor this to take a screenshots object where a format is configurable.

Patches are welcome, I'm guessing.

I'd change the third parameter, screenshotsPath (which comes from this.settings.screenshots.path) to take this.settings.screenshots instead and fix all the places that call it (recursively) and then document the new filename_format in a similar commit in nightwatch-docs's gettingstarted/configuration/config-test-settings.md

I'd let filename_format default to the current format with %T_%S where:

%T: Textual description

%S: Date string like Sep-29-2017-160341-GMT+0200

%I: Date string in ISO 8601 Notation like: 2008-09-15T15:53:00+05:00

That way we could configure file_format: '%I_%T' and get what we want without messing with the default. (Or perhaps the default should change?)

Are there other considerations to be taken into account? Anything preventing such a PR from being considered?

@beatfactor
Copy link
Member

The default shouldn't change. I think we could add the option to specify a function that would take the filename, date and any other relevant stuff as arguments and use the return value.

@pmorch
Copy link
Author

pmorch commented Feb 12, 2019

Yeah, I thought of that, but I dismissed it because I mistakenly thought that the nightwatch.conf.js config file was a .json file, not a full-blown JavaScript file.

Your suggestion is much better. The default value being a function that returns exactly the current file names.

@beaudamorevitas
Copy link

I would like to hard-code the name so it overwrites the last one every time for the reason that I want to insert the screenshot on fail into the report html via 64 bit encoding.

@Pieras2
Copy link

Pieras2 commented Mar 31, 2020

I need this too, as I get fail I don't know under which browser the test failed.
IMO there shall be a way to add user's custom prefix for file name.

@Pieras2
Copy link

Pieras2 commented Mar 31, 2020

quick workaround - define folder "screens" in NW config as e.g. screens_firefox (do it per browser config), at least screens will be separated by folder names.

I stopped counting on any improvement and fixes.

@kretschmannj
Copy link

+1 for adding this feature. The current default file name is really ugly ...

_FAILED_Mar-30-2021-140357-GMT-0700-(Pacific-Daylight.png

... and we need a way to set our own file name.

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

No branches or pull requests

5 participants