Skip to content

Add tests for filenames with surrogate pairs on Windows #51789

Closed
Closed
Add tests for filenames with surrogate pairs on Windows#51789
Labels
feature requestIssues requesting new Node.js features.windowsIssues and PRs related to the Windows platform.

Description

@AlttiRi
AlttiRi
opened on Feb 17, 2024

What is the problem this feature will solve?

That issue is still not fixed in LTS. There are no guarantees that the bug will not appear in Current again.

What is the feature you are proposing to solve the problem?

Just add tests for fs functions that will work with names contain surrogate pairs (emoji, but not only):

import fs from "node:fs/promises";

await fs.writeFile("🚀🔥🛸.txt", "");
await fs.lstat("🚀🔥🛸.txt");

Don't release Node.js while they fail.

What alternatives have you considered?

Something like this:
https://github.com/oven-sh/bun/blob/3221bfeeb7036e76872c1a602aa5cd7c83ed3b4a/test/js/node/fs/fs.test.ts#L681-L701

Activity

  1. AlttiRi
    added
    feature requestIssues requesting new Node.js features.
    on Feb 17, 2024
  2. github-project-automation
    added this to Node.js feature requestson Feb 17, 2024
  3. github-project-automation
    moved this to Pending Triage in Node.js feature requestson Feb 17, 2024

  4. mertcanaltin commented on Feb 17, 2024

    @mertcanaltin
    mertcanaltin
    on Feb 17, 2024
    Member

    I think a file name with emojis will not be healthy in development processes

  5. AlttiRi commented on Feb 17, 2024

    @AlttiRi
    AlttiRi
    on Feb 17, 2024
    Author

    Node.js is only for the "development process" now?

    What is the "development process"? And why while this "development process" I can't access the files with the legal filenames?

    Node.js must not be used for desktop (Electron, NW and console) applications?

    There is a file system bug on Windows.
    Fix it and add tests to prevent it from regression in future.
    If there were tests, then the breaking regression bug #48673 would not have appeared, which has not been fixed in LTS yet.

  6. anonrig commented on Feb 18, 2024

    @anonrig
    anonrig
    on Feb 18, 2024
    Member

    Any PRs are welcome

  7. mertcanaltin commented on Feb 18, 2024

    @mertcanaltin
    mertcanaltin
    on Feb 18, 2024
    Member

    Node.js is only for the "development process" now?

    What is the "development process"? And why while this "development process" I can't access the files with the legal filenames?

    Node.js must not be used for desktop (Electron, NW and console) applications?

    There is a file system bug on Windows.
    Fix it and add tests to prevent it from regression in future.
    If there were tests, then the breaking regression bug #48673 would not have appeared, which has not been fixed in LTS yet.

    I understand better now, sorry, I thought it was just a name feature with emojis.

  8. mertcanaltin commented on Feb 18, 2024

    @mertcanaltin
    mertcanaltin
    on Feb 18, 2024
    Member

    I'll be opening a pr trying to solve this

  9. mertcanaltin commented on Feb 18, 2024

    @mertcanaltin
    mertcanaltin
    on Feb 18, 2024
    Member

    I created a test for it and everything looks good
    If there is something missing or something you would like to see improved, please do not hesitate

  10. AlttiRi commented on Feb 18, 2024

    @AlttiRi
    AlttiRi
    on Feb 18, 2024
    Author

    Yeah, the test fails on 20.11.1 LTS and passes on 21.6.2 Current as expected.

    UPD.

    node-v20.11.1-x64.msi:
    20.11.1 LTS

    node-v21.6.2-x64.msi:
    21.6.2 Current

  11. mertcanaltin commented on Feb 18, 2024

    @mertcanaltin
    mertcanaltin
    on Feb 18, 2024
    Member

    I ran my test in the version you said and there was no problem. @AlttiRi

    v20.11.1
➜  node git:(dev-51789) node test/common/file-ops-with-surrogate-pairs-on-windows.js
▶ File operations with filenames containing surrogate pairs on Windows
  ✔ should write, read, and delete a file with surrogate pairs in the filename (19.227042ms)
▶ File operations with filenames containing surrogate pairs on Windows (20.333584ms)

ℹ tests 1
ℹ suites 1
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 25.379917
➜  node git:(dev-51789) ```

  12. anonrig commented on Feb 19, 2024

    @anonrig
    anonrig
    on Feb 19, 2024
    Member

    cc @nodejs/platform-windows

  13. anonrig
    added
    windowsIssues and PRs related to the Windows platform.
    on Feb 19, 2024
  14. richardlau
    closed this as completedin 8e7da60on Feb 28, 2024
  15. pluris
    added a commit that references this issue on Feb 28, 2024

    test: test surrogate pair filenames on windows

    8e7da60

  16. AlttiRi commented on Mar 11, 2024

    @AlttiRi
    AlttiRi
    on Mar 11, 2024
    Author

    Was this test backported to 20.x?
    Was the bug fixed on 20.x?

  17. richardlau commented on Mar 12, 2024

    @richardlau
    richardlau
    on Mar 12, 2024
    Member

    Was this test backported to 20.x? Was the bug fixed on 20.x?

    #51976

  18. richardlau
    added a commit that references this issue on Mar 15, 2024

    test: test surrogate pair filenames on windows

    e8f5735
  19. avivkeller
    removed this from Node.js feature requestson Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.windowsIssues and PRs related to the Windows platform.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions