test: do not use file names with illegal characters in tests #1421
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I can check this one out fine, but I don't get the expected error message when building: |
that's weird, the tests pass for me and I don't see any OS-dependent thing. If not, can you add a [edit: fix typo] |
I checked the man page just in case... 😉 Here's the full output from the tests: |
Interesting, it seems the file ending with full-stop was not checked out as-is, somehow the final full-stop character was removed… (by Windows git?). |
|
Did git give you any warning about that when checking out this branch? |
|
Also, what do you see in your file explorer? (I'm trying to figure out if the full-stop character was removed at check-out time or if it's a runtime issue) |
|
(by the way let me know if you don't have more time to spend on this, and I'll dust off my Windows VM 😊) |
|
Ah, just seeing that the Windows naming conventions say:
Mmm. I'm considering adding a step instruction to disable these tests on Windows. ( |
|
Sorry, didn't see the new emails come in while I was going through the rest of the PRs. No, there weren't any error messages from git when I checked it out. The weird thing is it doesn't recognize any files as missing after checkout, but it picks up the two files you mention as unversioned. This is immediately after deleting the branch and checking it out again, so it's nothing from the epubcheck build. |
Ya, I'd go with whatever is easiest. This is a pretty minor piece in the overall scheme of things. |
yeah, in fact this PR doesn't fix anything: files ending with a period are rejected like the ones with invalid characters before… it's just that you were not warned because you had disabled the NTFS protection setting earlier while we were troubleshooting the issue 😉 so back to square one. I'm going to try if a packaged EPUB works (i.e. can be unpacked without creating runtime trouble). |
File names with EPUB-illegal characters are also NTFS-invalid names, so they can cause issues (cannot be easily checked out by git on Windows or may cause runtime issues on NTFS). This commit replaces the expanded EPUBs in invalid characters by tests by packaged EPUBs. Fix #1420.
rdeltour
force-pushed
the
fix/test-illegal-ocf-filenames
branch
from
f83e394
to
2809c23
Compare
|
@mattgarrish I ended up using packaged EPUBs. It works fine on my Windows 10 box:
|
|
Ya, this obviously works for me now, too. |
mattgarrish
approved these changes
Dec 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
File names with EPUB-illegal characters are also NTFS-invalid names, so they can cause issues (cannot be easily checked out by git on Windows or may cause runtime issues on NTFS).
This commit replaces tests using files with invalid characters by tests using files with a full stop as the last character. Other illegal characters are tested in the file name checker unit tests.
Fix #1420.