Kenzitron/protractor-jasmine2-html-reporter

screenshotsFolder doesn't work

Closed this issue · 8 comments

When i try to config the screenshotsFolder property, the generated folder name is always concatenated with the savePath folder name and always on the same level.

htmlReporter = new Jasmine2HtmlReporter({
savePath: 'tests/e2e/screenshots',
screenshotsFolder: 'images',
fixedScreenshotName: true
})

Result:
tests/e2e/screenshots
tests/e2e/screenshotsimages

+1

@carantes Change Line Number 167 index.js to this:

screenshotPath = path.join(self.savePath + '/' + self.screenshotsFolder, spec.screenshot);

Pull Request Submitted @carantes @Kenzitron

I would like to point that in the readme you see the savePath value always with '/' at the end, that makes the folder structure work. If everyone is using that notation, when we introduce this change will affect them.

@carantes, have you tried with the next configuration?:

htmlReporter = new Jasmine2HtmlReporter({
savePath: 'tests/e2e/screenshots/',
screenshotsFolder: 'images',
fixedScreenshotName: true
})

If that works I should close the PR without accepting it. Anyways, @stenmuchow thanks you so much for your collaboration and support :)

@Kenzitron Sounds good, close away... But it seems that this part is not very intuitive and would cause confusion again in the future.

In my opinion the code needs to be more defensive that it currently is - accepting both ways of defining a path with and without the '/.'

@Kenzitron @stenmuchow thanks for the help, i forgot the slash "/" on the end of savePath.

Ok, maybe the pull request should be in that line. check if we have the /
at the end and include it or remove it accordingly.

Do you agree?

On Mon, Feb 29, 2016 at 9:56 PM, Carlos Eduardo Arantes Ferreira <
notifications@github.com> wrote:

Closed #24
#24
.


Reply to this email directly or view it on GitHub
#24 (comment)
.

I think it can only help to make the user’s life easier!

On Feb 29, 2016, at 9:23 PM, Kenzitron notifications@github.com wrote:

Ok, maybe the pull request should be in that line. check if we have the /
at the end and include it or remove it accordingly.

Do you agree?

On Mon, Feb 29, 2016 at 9:56 PM, Carlos Eduardo Arantes Ferreira <
notifications@github.com> wrote:

Closed #24
#24
.


Reply to this email directly or view it on GitHub
#24 (comment)
.


Reply to this email directly or view it on GitHub #24 (comment).