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

test-fs-promises-write-optional-params does't check read bytes counts #46144

Closed
dmitry-chukanov opened this issue Jan 9, 2023 · 2 comments · Fixed by #46238
Closed

test-fs-promises-write-optional-params does't check read bytes counts #46144

dmitry-chukanov opened this issue Jan 9, 2023 · 2 comments · Fixed by #46238
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. fs Issues and PRs related to the fs subsystem / file system.

Comments

@dmitry-chukanov
Copy link

Test

test-fs-promises-write-optional-params

Platform

Linux x64

Console output

No response

Build links

Additional information

There are several options in test-fs-promises-write-optional-params (at the file end) such as 'undefined', 'null', '{}' and so on. And for '{}' option we read 0 bytes in testValid() function and read buffer hasn't any changes after read. But all asserts in testValid() functions are OK, because 2 errors:

  1. No any assert for really read bytes (may be 0 or any other positive value). In the test string assert.ok(writeResult.bytesWritten >= readResult.bytesRead); is ok because 3>0
  2. Only one buffer without any resets after write is used for write/read operations. This buffer doesn't have any changes after read if 0 bytes was read and there are the same values that before write. So assert.deepStrictEqual(writeResult.buffer, readResult.buffer);is also ok, because both buffers are the same and because read doesn't make any changes in buffer if 0 bytes was read.
@dmitry-chukanov dmitry-chukanov added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jan 9, 2023
@github-actions github-actions bot added the linux Issues and PRs related to the Linux platform. label Jan 9, 2023
@dmitry-chukanov
Copy link
Author

dmitry-chukanov commented Jan 10, 2023

The same test behavior is for test-fs-write-optional-params

@LiviaMedeiros LiviaMedeiros added fs Issues and PRs related to the fs subsystem / file system. and removed linux Issues and PRs related to the Linux platform. labels Jan 17, 2023
@LiviaMedeiros
Copy link
Contributor

Thanks for catching this! The same also applies to test-fs-write-sync-optional-params.

  1. This part of test checks compatibility with read counterparts and for some combinations (e.g. { length: null }) we can't get equal bytesWritten and bytesRead, hence the "greater or equal" assertion. Assertion when there is an explicit length is indeed missing.
  2. There is an issue in these tests: when position == null, subsequent read starts from current file position rather than from beginning.

Both errors should be fixed by linked PR, but suggestions or alternative PR are always welcome.

RafaelGSS pushed a commit that referenced this issue Jan 20, 2023
PR-URL: #46238
Fixes: #46144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol pushed a commit that referenced this issue Mar 3, 2023
PR-URL: #46238
Fixes: #46144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol pushed a commit that referenced this issue Mar 5, 2023
PR-URL: #46238
Fixes: #46144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants