Fix --test-probe #1365
Fix --test-probe #1365
Conversation
1. Changes `folder` to `dir.to_s` (`folder` no longer exists). 2. Changes `passing.any?` to `!passing.empty?`, as `any?` has pitfalls: Took me a few minutes to debug when I tried with `input_path` that was returning `nil` (`[nil].any? #=> false`).
Also: 1. Moves the `normalize*` functions and `rtrim` to a separate object file, so that they can be unit tested quickly and easily. 2. Changes `string_to_output` to also replace `\r\n`, fixing Windows tests. 3. Improves other `normalize_*` functions and adds tests. Refs sass#2843 sass-spec PRs: 1. Bug fixes to make it run: sass/sass-spec#1365 2. Enabling the newly passing tests: sass/sass-spec#1366
Also: 1. Moves the `normalize*` functions and `rtrim` to a separate object file, so that they can be unit tested quickly and easily. 2. Changes `string_to_output` to also replace `\r\n`, fixing Windows tests. 3. Improves other `normalize_*` functions and adds tests. Refs #2843 sass-spec PRs: 1. Bug fixes to make it run: sass/sass-spec#1365 2. Enabling the newly passing tests: sass/sass-spec#1366
|
Why does this change |
|
The original bug (sass/libsass#1774) this test was added for involved the interaction of Windows command line path handling (some arcane Windows path NFC normalization stuff); it should be tested with real files, and the actual test uses sassc binary directly for this one (in addition to testing via sass-spec). |
|
HRX tests are run with real files. The test runner creates a directory named after the HRX file and invokes the Sass compiler against that directory. |
|
Right but this test also runs using a real sassc binary (in addition to running via sass-spec). |
|
LibSass's specs shouldn't have such a tightly-coupled dependency on the specific structure of this repo—that's liable to break. I strongly recommend just creating a matching directory in the LibSass repo directly. That's what we do for Dart Sass tests that exercise details of the CLI. |
|
I can open a corresponding issue in the libsass repo to do this. |
|
Can you add a TODO to the spec to re-HRXify it when the LibSass issue is fixed? |
|
Opened sass/libsass#2854 and added a TODO there (this way re-hrxifying will be a simple git revert, won't need to revert the TODO as well) |
|
I'd really rather have a TODO here too, so someone doesn't accidentally change this without realizing it'll break LibSass. |
|
OK, I'll amend the commit and post here again when done |
These 2 files are used to test UTF8 paths directly with sassc executable, requiring a real directory. This special handling is required to test fully on Windows (Ruby wrapper can mask bugs), see sass/libsass#1774.
|
Amended |
As sass-spec is migrating to HRX, we need to copy this test here so we can run it directly via `sassc` instead of the sass-spec runner. Refs sass#2854 Refs sass/sass-spec#1365
As sass-spec is migrating to HRX, we need to copy this test here so we can run it directly via `sassc` instead of the sass-spec runner. Refs #2854 Refs sass/sass-spec#1365
foldertodir.to_s(folderno longer exists).passing.any?to!passing.empty?, asany?has pitfalls:Took me a few minutes to debug when I tried with
input_pathinstead ofdir.to_s, and that was returningnil([nil].any? #=> false).