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

Replace Stream.readable.pipe() with data event API in getHash() #4292

Closed

Conversation

seaoak
Copy link
Member

@seaoak seaoak commented May 4, 2020

What does it do?

Fix one of errors of the issue #3818. (#3818 (comment))

With Node.js v10.x, npm test fails some times.
One of them is process() - skip (mtime changed but hash matched): as follows:

  1) Hexo
       Box
         Box
           process() - skip (mtime changed but hash matched):
     AssertError: expected spy to be called with match
[_File] {
  params: {  },
  path: "a.txt",
  source: "J:\00_Seaoak\hexo\simplify_codeblock_escape\hexo\test\scripts\box\box_tmp\test\a.txt",
  type: "update"
} { path: "a.txt", type: "skip" }
      at Object.fail (J:\00_Seaoak\hexo\simplify_codeblock_escape\hexo\node_modules\sinon\lib\sinon\assert.js:107:21)
      at failAssertion (J:\00_Seaoak\hexo\simplify_codeblock_escape\hexo\node_modules\sinon\lib\sinon\assert.js:66:16)
      at Object.assert.(anonymous function) [as calledWithMatch] (J:\00_Seaoak\hexo\simplify_codeblock_escape\hexo\node_modules\sinon\lib\sinon\assert.js:92:13)
      at Context.it (J:\00_Seaoak\hexo\simplify_codeblock_escape\hexo\test\scripts\box\box.js:160:17)

This patch fixes this error.

I found that a combination Stream.readable.pipe() and Crypt.createHash() causes this error.
By replacing Stream.readable.pipe() with data event API, this error is resolved.

But I can not explain why this replacing does resolve this error.

How to test

nvm use 10.20.1
git clone -b workaround/fs.readable.pipe_not_work_well https://github.com/seaoak/hexo.git
cd hexo
npm install
npm test

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 97.743% when pulling 117e01f on seaoak:workaround/fs.readable.pipe_not_work_well into 714e752 on hexojs:master.

@segayuu
Copy link
Contributor

segayuu commented May 19, 2020

Hash Stream Mode already uses Hash#digest() once when issuing the finish event. This behavior is correct when sending a hash value by connecting the stream with Hash#pipe(), but you cannot use Hash#digest() again in the 'finish' event.

@seaoak
Copy link
Member Author

seaoak commented May 20, 2020

@segayuu Thank you for your information!

I understand the cause of errors, and make a new PR #4323 (replace this draft PR).

@seaoak seaoak closed this May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants