search

Kenzitron / protractor-jasmine2-html-reporter

HTML reporter for Jasmine2 and Protractor
32 stars 53 forks source link

screenshotsFolder doesn't work #24

Closed carantes closed 8 years ago

carantes commented 8 years ago

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

s10mcow commented 8 years ago

+1

s10mcow commented 8 years ago

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

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

s10mcow commented 8 years ago

Pull Request Submitted @carantes @Kenzitron

Kenzitron commented 8 years ago

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

s10mcow commented 8 years ago

@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 '/.'

carantes commented 8 years ago

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

Kenzitron commented 8 years ago

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 https://github.com/Kenzitron/protractor-jasmine2-html-reporter/issues/24 .

— Reply to this email directly or view it on GitHub https://github.com/Kenzitron/protractor-jasmine2-html-reporter/issues/24#event-571826570 .

s10mcow commented 8 years ago

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 https://github.com/Kenzitron/protractor-jasmine2-html-reporter/issues/24 .

— Reply to this email directly or view it on GitHub https://github.com/Kenzitron/protractor-jasmine2-html-reporter/issues/24#event-571826570 .

— Reply to this email directly or view it on GitHub https://github.com/Kenzitron/protractor-jasmine2-html-reporter/issues/24#issuecomment-190402674.