CI: Enable test/run-unittest.sh on Windows #2689
Merged
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
|
✅ PR OK, no changes in deprecations or warnings Total deprecations: 14 Total warnings: 0 Build statistics: statistics (-before, +after)
executable size=5280256 bin/dub
rough build time=77sFull build output |
kinke
force-pushed
the
win_enable_unittest.sh
branch
2 times, most recently
from
28cdc32
to
97e83f3
Compare
kinke
commented
Aug 18, 2023
| @@ -39,10 +39,10 @@ FRONTEND="${FRONTEND:-}" | |||
|
|
|||
| if [ -z ${FRONTEND:-} ]; then | |||
| if [ "$DC_BIN" == "ldc2" ]; then | |||
| FRONTEND=$(ldc2 --version | grep -Po "based on DMD v\K(2\.\d+\.\d)") | |||
| FRONTEND=$(ldc2 --version | grep 'based on DMD v2.' | sed -E -n 's/^.*DMD v(2\.[0-9]+\.[0-9]).*$/\1/p') | |||
There was a problem hiding this comment.
The grep in my Windows VM complained about grep -P only working for some locales.
kinke
commented
Aug 18, 2023
test/issue2452/.no_test
Outdated
There was a problem hiding this comment.
This file is needed because the path of the unittest runner executable on Windows exceeds the 260 allowed chars... but there aren't any unittests to run either and no extra dub config, so skip that seemingly superfluous test build altogether.
kinke
commented
Aug 18, 2023
| rm -rf test/issue130-unicode-СНА* | ||
| # ImportC probably requires set-up MSVC environment variables | ||
| rm -rf test/use-c-sources | ||
| fi |
There was a problem hiding this comment.
Not sure if there's a more elegant way to handle this, or making DMD's limitations on Windows more prominent.
kinke
force-pushed
the
win_enable_unittest.sh
branch
from
97e83f3
to
c555639
Compare
shoo
reviewed
Aug 26, 2023
Comment on lines
+96
to
+107
| dub --single test/run-unittest.d | ||
|
|
||
| # FIXME: DMD fails a few tests on Windows; remove them for now | ||
| if [[ '${{ matrix.dc }}' = dmd* ]]; then | ||
| # DLL support is lacking | ||
| rm -rf test/{1-dynLib-simple,2-dynLib-dep,2-dynLib-with-staticLib-dep} | ||
| # Unicode in paths too | ||
| rm -rf test/issue130-unicode-СНА* | ||
| # ImportC probably requires set-up MSVC environment variables | ||
| rm -rf test/use-c-sources | ||
| fi | ||
| test/run-unittest.sh |
There was a problem hiding this comment.
I have worked on this subject in the past. Check out my outcomes below:
master...shoo:dub:enh_test_windows
The difference from this PR's approach is that in my approach I changed run-unittest.d to be more like run-unittest.sh and not dependent on .sh .
In windows, there are several ways to run .sh . Cygwin(or MSYS2) / git-bash / WSL
GitHub Actions uses git-bash, which I think should be avoided for compatibility reasons.
There was a problem hiding this comment.
AFAIK, the gnu tools bundled with git for windows originate from msys2. Why do you think it should be avoided for compatibility reasons? Since almost any Windows dev has git, he/she has bash + gnu tools too. I've been using bash on Windows for years, and it's absolutely okay, especially for such trivial stuff that is easier handled in bash than D.
|
Ping, why does this have to wait for so long? It significantly increases test coverage on Windows. |
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.
No description provided.