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

Filename limitation in cy.screenshot #3052

Closed
ignoreswing opened this issue Jan 3, 2019 · 4 comments
Closed

Filename limitation in cy.screenshot #3052

ignoreswing opened this issue Jan 3, 2019 · 4 comments
Assignees
Labels
pkg/server This is due to an issue in the packages/server directory type: unexpected behavior User expected result, but got another
Milestone

Comments

@ignoreswing
Copy link

ignoreswing commented Jan 3, 2019

Description:

I was wondering if Cypress could allow CJK's characters in a filename when we call cy.screenshot() or cy.screenshot(filename).

As the definition of source code screenshots.coffee (if this is the right place?), the valid characters in a filename seems like

invalidCharsRe = /[^0-9a-zA-Z-_\s\(\)]/g

Besides the valid characters, all the other characters in a filename will replace to "".

replaceInvalidChars = (str) ->
  str.replace(invalidCharsRe, "")

We wrote Chinese description for each test run, for we can generate Chinese report. This cause problems when we use default naming convention cy.screenshot(), or even worse, cy.screenshot(Chinese_Filename), as described below.

Current behavior:

// spec name = test.spec.js

context('語言', () => { // language
    it('預設', function() { // default naming rule
        cy.screenshot();
        // screenshots/test.spec.js/ -- .png
        // Will create a image named " -- .png" under the folder "test.spec.js"
    });
    it('Give shot a name', function() { // Take screen by giving a Chinese filename
        cy.screenshot('你好');
        // screenshots/test.spec.js.png
        // Will create a image named "test.spec.js.png" and
        // the created image is NOT under the folder "test.spec.js"
    });
});

Desired behavior:

context('語言', () => {
    it('預設', function() {
        cy.screenshot(); // screenshots/test.spec.js/語言 -- 預設.png
    });
    it('Give shot a name', function() {
        cy.screenshot('你好'); // screenshots/test.spec.js/你好.png
    });
});

Something I have tried

Listen the event after:screenshot, and then, I can move the created image to the correct place with correct filename. Cypress.Screenshot.defaults({onAfterScreenshot ($el, props) {},}) can do the same job too. However, it will lose the information corresponding to test run's description in the above manner. This is not working if I take screenshot via default naming rulecy.screenshot(). I think this is just a workaround.

Is it possible to only discard the special characters in a filename without losing CJK's characters? Though, this would become not fully portable filenames, but it become more convenience for non-english users.

Versions

Cypress 3.1.2
MacOS 10.13.4
Chrome 71.0.3578.98

@jennifer-shehane jennifer-shehane added type: unexpected behavior User expected result, but got another stage: ready for work The issue is reproducible and in scope pkg/server This is due to an issue in the packages/server directory difficulty: 1️⃣ labels Jan 3, 2019
@jennifer-shehane
Copy link
Member

@lilaconlee Also an issue to possibly look at.

@ghost
Copy link

ghost commented Jan 11, 2019

i also met the same problem when use cy.screenshot() to save file name in chinese.

@lilaconlee
Copy link
Contributor

Updated the screenshot filename sanitation to use sanitize-filename instead of this regex.

There were some changes in kinda unrelated tests as $, ^, and % are now allowed in filenames. According to the package, this should be okay but lemme know if these were excluded for a reason.

@jennifer-shehane jennifer-shehane added stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope labels Jan 22, 2019
chrisbreiding pushed a commit that referenced this issue Jan 22, 2019
- Fixes #3052 

From the issue
> There were some changes in kinda unrelated tests as $, ^, and % are now allowed in filenames. According to the package, this should be okay but lemme know if these were excluded for a reason.
@chrisbreiding chrisbreiding added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Jan 22, 2019
@chrisbreiding chrisbreiding added this to the Sprint 20 milestone Jan 22, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 30, 2019

Released in 3.1.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/server This is due to an issue in the packages/server directory type: unexpected behavior User expected result, but got another
Projects
None yet
Development

No branches or pull requests

4 participants