fix: toc test fails due to separator on Windows #88

AyumuTakai · 2020-12-10T14:49:26Z

Windowsではpath.relativeで付与されるセパレータが<a href="..¥test.html">のように"¥"になるためテストに失敗します。
生成されたtoc.htmlをMacにコピーしてChromeで試した限りでは"/"でも"¥"でも区切りとして機能しているようなのでテストで引っ掛るところ以外は修正しなくても実用上は問題ないと思います。

import path form 'upath'; の順番が変わってしまったのはpretty-quickによる自動フォーマットの結果です。

Windowsではpath.relativeで付与されるセパレータが<a href="..\test.html">のように"\"になるためテストに失敗する。生成されたtoc.htmlをMacにコピーしてChromeで試した限りでは"/"でも"\"でも区切りとして機能しているようなので修正しなくても実用上は問題ないと思われる。

spring-raining

Windows環境が無いため、他の方のReviewを待ちたいです。
ところで、path.relative は他のコードでも多数利用されていますが、この箇所以外の変更は不要でしょうか？

MurakamiShinyu · 2020-12-16T09:57:58Z

Windows環境あるので、見てみます。

upath はWindowsでもパス区切りを '/' で扱うようにするものです。

MurakamiShinyu · 2020-12-16T11:00:09Z

upathは関数の引数も戻り値も toUnix() で '/' 区切りに直すので、.posix にしなくてもよいはずなのに、なぜだめだったのか、調べます。
https://github.com/anodynos/upath/blob/master/source/code/upath.coffee#L18-L35

AyumuTakai · 2020-12-16T14:35:53Z

再度テストを実行してみました。
developブランチ最新コミット 41aee32

upath@^1.2.0:
  version "1.2.0"
  resolved "https://registry.yarnpkg.com/upath/-/upath-1.2.0.tgz#8f66dbcd55a883acdae4408af8b035a5044c1894"
  integrity sha512-aZwGpamFO61g3OlfT7OQCHqhGnW43ieH9WZeP7QxN/G/jS4jfqUkZxoryvJgVPEcrl5NL/ggHsSmLMHuH64Lhg==

Microsoft Windows [Version 10.0.18363.1256]
コマンドプロンプト

 FAIL  tests/toc.test.ts
  ● generateToC

    expect(received).toBe(expected) // Object.is equality

    Expected: "<html><head><title>Table of Contents</title><link href=\"manifest.json\" rel=\"manifest\" type=\"application/webpub+json\"></head><body><nav id=\"toc\" role=\"doc-toc\"><ul><li><a href=\"../test.html\">Title</a></li></ul></nav></body></html>"
    Received: "<html><head><title>Table of Contents</title><link href=\"manifest.json\" rel=\"manifest\" type=\"application/webpub+json\"></head><body><nav id=\"toc\" role=\"doc-toc\"><ul><li><a href=\"..\\test.html\">Title</a></li></ul></nav></body></html>"

Git for Windows付属のbash

 expect(received).toBe(expected) // Object.is equality

    Expected: "<html><head><title>Table of Contents</title><link href=\"manifest.json\" rel=\"manifest\" type=\"application/webpub+json\"></head><body><nav id=\"toc\" role=\"doc-toc\"><ul><li><a href=\"../test.html\">Title</a></li></ul></nav></body></html>"
    Received: "<html><head><title>Table of Contents</title><link href=\"manifest.json\" rel=\"manifest\" type=\"application/webpub+json\"></head><body><nav id=\"toc\" role=\"doc-toc\"><ul><li><a href=\"..\\test.html\">Title</a></li></ul></nav></body></html>"

開発環境をようやくWindowsにも入れたのでこれから少し調べてみます。

MurakamiShinyu · 2020-12-16T14:39:06Z

分かりました。upathの最新版で問題は修正されています。古いupathのこの問題でした。
anodynos/upath#40

6a4ab12 を取り消して、upath を最新(2.0.1)にすれば解決します。

MurakamiShinyu

'path' のかわりに 'upath' を使う理由はWindowsでのパス区切りによる問題をなくすためです。ところがupath 2.0.0までjestから使われたときに 'upath' の関数がただの 'path' の関数に変わってしまう不具合があったために、testに失敗していました。 posix. を入れたのは元に戻して、この問題が解決されている最新版のupath 2.0.1に更新してください。

AyumuTakai · 2020-12-16T15:04:43Z

upathをアップデートすることで問題が解決することを確認しました。
PRのほうも修正してコミットしました。Windows、Macともにtoc.test.tsのテストに通っています。

spring-raining

package.jsonが更新されているため、yarn installを再度実行してyarn.lockファイルの更新も含めてください。

AyumuTakai · 2020-12-17T04:01:21Z

yarn.lock を追加しました。

package関係を変更した場合はyarn.lockを忘れないようにします。

spring-raining

良さそうです！

AyumuTakai · 2020-12-17T04:08:35Z

マージありがとうございます。
@MurakamiShinyu さんにも調査いただきありがとうございました。
自分ではたぶん原因のバグまで到達できませんでした。

MurakamiShinyu · 2020-12-17T06:04:53Z

前にWindowsでvivliostyle-cliが動かなかったのを調べて直すのに、upath を使うことにしました： fix: Errors due to path problem on Windows (#69)。それから upath の中身をみて少し直したりもした(anodynos/upath#38 )ので upath の動作について知ってました。

upath使用で ..\test.html のようなbackslash付きのパスにはならなくなったはずなのに変だなと思って調べたら、最新の upath v2.0.1 で修正されたばかりの問題 Proxied methods don't work in jest ずばりでした。

AyumuTakai requested review from spring-raining and uetchy as code owners December 10, 2020 14:49

spring-raining reviewed Dec 16, 2020

View reviewed changes

MurakamiShinyu requested changes Dec 16, 2020

View reviewed changes

fix: remove .posix and update upath 2.0.1

594b8dd

MurakamiShinyu approved these changes Dec 16, 2020

View reviewed changes

spring-raining requested changes Dec 16, 2020

View reviewed changes

fix: include yarn.lock

605897e

spring-raining approved these changes Dec 17, 2020

View reviewed changes

spring-raining merged commit 480868f into vivliostyle:develop Dec 17, 2020

AyumuTakai deleted the fix/toc-url-separator branch December 17, 2020 04:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: toc test fails due to separator on Windows #88

fix: toc test fails due to separator on Windows #88

AyumuTakai commented Dec 10, 2020 •

edited

Loading

spring-raining left a comment

MurakamiShinyu commented Dec 16, 2020 •

edited

Loading

MurakamiShinyu commented Dec 16, 2020

AyumuTakai commented Dec 16, 2020 •

edited

Loading

MurakamiShinyu commented Dec 16, 2020

MurakamiShinyu left a comment

AyumuTakai commented Dec 16, 2020

spring-raining left a comment

AyumuTakai commented Dec 17, 2020

spring-raining left a comment

AyumuTakai commented Dec 17, 2020

MurakamiShinyu commented Dec 17, 2020

fix: toc test fails due to separator on Windows #88

fix: toc test fails due to separator on Windows #88

Conversation

AyumuTakai commented Dec 10, 2020 • edited Loading

spring-raining left a comment

Choose a reason for hiding this comment

MurakamiShinyu commented Dec 16, 2020 • edited Loading

MurakamiShinyu commented Dec 16, 2020

AyumuTakai commented Dec 16, 2020 • edited Loading

MurakamiShinyu commented Dec 16, 2020

MurakamiShinyu left a comment

Choose a reason for hiding this comment

AyumuTakai commented Dec 16, 2020

spring-raining left a comment

Choose a reason for hiding this comment

AyumuTakai commented Dec 17, 2020

spring-raining left a comment

Choose a reason for hiding this comment

AyumuTakai commented Dec 17, 2020

MurakamiShinyu commented Dec 17, 2020

AyumuTakai commented Dec 10, 2020 •

edited

Loading

MurakamiShinyu commented Dec 16, 2020 •

edited

Loading

AyumuTakai commented Dec 16, 2020 •

edited

Loading