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

screenshotsFolder not working correctly #1525

Closed
MysteriousNothing opened this issue Apr 3, 2018 · 6 comments
Closed

screenshotsFolder not working correctly #1525

MysteriousNothing opened this issue Apr 3, 2018 · 6 comments
Assignees
Labels
topic: screenshots 📸 type: enhancement Requested enhancement of existing feature type: unexpected behavior User expected result, but got another

Comments

@MysteriousNothing
Copy link

MysteriousNothing commented Apr 3, 2018

Current behavior:

Using screenshotsFolder does not save fails to correct folder when using cy.screenshot

Cypress.config("screenshotsFolder", "cypress/screenshots/desktop") 
expect(Cypress.config('screenshotsFolder')).to.eq("cypress/screenshots/desktop")
cy.screenshot('homepage_desktop')

Expected passes the test, but screenshots are saved to wrong folder cypress/screenshots.

Desired behavior:

Using screenshotsFolder on a test should save fails correct folder

How to reproduce:

it('Homepage test', function () {
  cy.wrap(lang_array).each(function(lang, i, array) {
    cy.viewport(1680, 1050)
    Cypress.config("screenshotsFolder", "/home/osboxes/<project>/<project-folder>/cypress/screenshots/desktop")
    Cypress.config('screenshotsFolder')
    expect(Cypress.config('screenshotsFolder')).to.eq(picutres_url+"/desktop")
    cy.visit('/?lang='+lang)
    cy.screenshot('homepage_desktop_'+lang)
    cy.viewport(720, 1280)
    Cypress.config("screenshotsFolder", "/home/osboxes/<project>/<project-folder>/cypress/screenshots/medium")
    expect(Cypress.config('screenshotsFolder')).to.eq(picutres_url +"/medium")
    Cypress.config("screenshotsFolder", picutres_url+'/medium')
    cy.screenshot('homepage_medium_'+lang)
    cy.viewport(320, 480)
    Cypress.config("screenshotsFolder", "/home/osboxes/<project>/<project-folder>/cypress/screenshots/mobile")
    expect(Cypress.config('screenshotsFolder')).to.eq(picutres_url +"/mobile")
    cy.viewport(320, 480)
    Cypress.config('userAgent', 'Mozilla/5.0 (iPhone; CPU iPhone OS 6_0 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Version/6.0 Mobile/10A5376e Safari/8536.25')
    cy.screenshot('1 homepage_mobile_'+lang)
  })
})
  • Operating System:
    DISTRIB_RELEASE=16.04
    DISTRIB_CODENAME=xenial
    DISTRIB_DESCRIPTION="Ubuntu 16.04.4 LTS"
  • Cypress Version: 5.8.0
  • Browser Version: FireFox 59.02
@jennifer-shehane jennifer-shehane added the stage: needs investigating Someone from Cypress needs to look at this label Apr 3, 2018
@jennifer-shehane
Copy link
Member

So, this is definitely what you are experiencing. However, I believe this is because not all of the configuration options are mutable at runtime with Cypress.config(). See #509 (comment)

But also, this is incredibly confusing if this is the case because:

  1. No errors are apparent when doing this
  2. The assertion expect(Cypress.config('screenshotsFolder')).to.eq("cypress/screenshots/desktop") actually passes
  3. The Cypress.config() when logged to the console also confirms the updated value.

Either way, I think you should be able to change the screenshotsFolder from within Cypress.config() I can definitely see the use case just from your example.

@MysteriousNothing
Copy link
Author

MysteriousNothing commented Apr 4, 2018

Girl, I'm more confused now! I have nothing in cypress.json file.

What I want to do is specify screenshot folder on the viewport, if mobile save to the mobile folder etc.

I don't understand what you want to say with this other post you linked, it seems fixed.

Using Cypress.config() displays all defaults values and last set screenshotsFolder Cypress.config("screenshotsFolder", "cypress/screenshots/mobile")

I want to note that Cypress.config("baseUrl", url) what I have worked just fine (default value is null) and I overwrite this with Cypress.config("baseUrl", url) in my test.

Entering the screenshotFolder cypress.json file fixes it for one viewport, I can't use it.

Test code in my previous comment is what I use to run my test.

Clicking on assert and screenshot after test is finished returns this in console :

cypress_runner.js:139344 Command:   assert
cypress_runner.js:139344 Actual:    cypress/screenshots/mobile
cypress_runner.js:139344 Expected:  cypress/screenshots/mobile
cypress_runner.js:139344 Message:   expected cypress/screenshots/mobile to equal cypress/screenshots/mobile
Command:  screenshot
cypress_runner.js:139344 Saved:    /home/osboxes/<projects>/<project-folder>/cypress/screenshots/6_iseteenindus_login_mobile_menu_open_et_EE.png
cypress_runner.js:139344 Size:     213.42 kB

As can you see it save to wrong folder.

@jennifer-shehane
Copy link
Member

Sorry, yes, I should have been clearer. I believe that Cypress needs to do work on our end to make this work and that Cypress should do work to support this feature. I do not think there is a workaround for you to achieve what you're trying to do today.

@brian-mann
Copy link
Member

Although we provide to you screenshotsFolder, it is not mutable. It's only ever once taken into account when Cypress first starts. From there it's just a dummy value you can access from your tests but you can't change.

We probably could make it mutable - and better yet, we should automatically throw if you try to set configuration values which are ignored from inside of the tests, so at least it's clear. I believe there's another issue open somewhere about this.

I think to solve your problem we will automatically nest screenshots within the screenshotsFolder with the same structure as your tests.

@brian-mann brian-mann self-assigned this May 24, 2018
@brian-mann brian-mann added type: enhancement Requested enhancement of existing feature and removed stage: needs investigating Someone from Cypress needs to look at this labels May 24, 2018
@brian-mann brian-mann added this to the 3.0.0 milestone May 24, 2018
@jennifer-shehane
Copy link
Member

We have been having some discussion recently on allowing passing a directory to the screenshots command. See #1771

This would clean up the code in your original issue to the following:

let lang_array = ['et_EE', 'en_US', 'ru_RU']

it('Homepage test', function () {
  cy.wrap(lang_array).each(function (lang, i) {
    cy.viewport(1680, 1050)
    cy.visit(`/?lang=${lang}`)
    cy.screenshot(`desktop/homepage_desktop_${lang}`)

    cy.viewport(720, 1280)
    cy.screenshot(`medium/homepage_medium_${lang}`)

    cy.viewport(320, 480)
    Cypress.config('userAgent', 'Mozilla/5.0 (iPhone; CPU iPhone OS 6_0 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Version/6.0 Mobile/10A5376e Safari/8536.25')
    cy.screenshot(`mobile/1 homepage_mobile_${lang}`)
  })
})

@jennifer-shehane
Copy link
Member

I am closing this issue in light of #1771 being delivered and I have also opened #3422 to track more specifically throwing when trying to set non mutable configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: screenshots 📸 type: enhancement Requested enhancement of existing feature type: unexpected behavior User expected result, but got another
Projects
None yet
Development

No branches or pull requests

3 participants