[RFC] test: win: enable remaining functional tests #8378
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
janlazo
force-pushed
the
win_functional_tests
branch
from
0e035c2
to
98858d6
Compare
janlazo
force-pushed
the
win_functional_tests
branch
2 times, most recently
from
5bf35f1
to
2ecade9
Compare
janlazo
force-pushed
the
win_functional_tests
branch
3 times, most recently
from
1482047
to
6d1ff98
Compare
|
I can't switch out cmake generators without deleting |
|
To clear The travis builds were failing because gnu.org is down, ff19f08 should fix it. |
janlazo
force-pushed
the
win_functional_tests
branch
2 times, most recently
from
7b42b75
to
304c16d
Compare
janlazo
commented
Oct 19, 2018
appveyor.yml
Outdated
| @@ -18,6 +18,7 @@ build_script: | |||
| - powershell ci\build.ps1 | |||
| cache: | |||
| - C:\msys64\var\cache\pacman\pkg -> ci\build.ps1 | |||
| - .deps -> ci\build.ps1 | |||
There was a problem hiding this comment.
Need to invalidate .deps/ to change the CMake generator
|
janlazo
commented
Oct 19, 2018
runtime/autoload/provider/node.vim
Outdated
| " XXX: The following code is not portable | ||
| " https://github.com/yarnpkg/yarn/issues/2049#issuecomment-263183768 | ||
| if has('unix') | ||
| let yarn_default_path = $HOME . '/.config/yarn/global/' . yarn_opts.entry_point |
There was a problem hiding this comment.
I don't use yarn so I won't fix this for Windows. I'm interested only in missing_provider for the nodejs functional tests.
There was a problem hiding this comment.
fwiw it's technically not non-portable code, as it'll still run without erroring on non-unix but not wind up taking the fast path.
janlazo
commented
Oct 19, 2018
runtime/autoload/provider/node.vim
Outdated
| return yarn_default_path | ||
| endif | ||
| endif | ||
| let yarn_opts.job_id = jobstart('yarn global dir', yarn_opts) |
There was a problem hiding this comment.
yarn uses shims so revert to the string format.
janlazo
commented
Oct 19, 2018
runtime/autoload/provider/node.vim
Outdated
| if executable('npm') | ||
| let npm_opts = deepcopy(s:NodeHandler) | ||
| let npm_opts.entry_point = '/neovim/bin/cli.js' | ||
| let npm_opts.job_id = jobstart('npm --loglevel silent root -g', npm_opts) |
There was a problem hiding this comment.
npm uses shims so revert to the string format.
|
If this fixes a bug for nodejs on Windows let's merge it quickly. @mqudsi can you review the changes? |
|
lgtm |
|
This fails the build on mingw. msvc build ignores this and runs the oldtests anyway. Why? |
janlazo
commented
Oct 20, 2018
| @@ -1,4 +1,5 @@ | |||
| Set-PSDebug -Trace 1 | |||
| $ErrorActionPreference = 'stop' | |||
There was a problem hiding this comment.
This caught the cmake error for mingw build but not MSVC build.
When does 'Running functional tests failed with error' appear in a failed MSVC build for nvim functional tests?
|
f95e0ae from #9087 broke the MSVC hack. Is there a cmake variable to modify MSBuild behavior and set IgnoreStandardErrorWarningFormat similar to |
janlazo
commented
Oct 20, 2018
janlazo
commented
Oct 21, 2018
| @@ -429,7 +429,7 @@ describe("'scrollback' option", function() | |||
| screen:expect{any='%$'} | |||
There was a problem hiding this comment.
What's the expected startup time for this pass?
There was a problem hiding this comment.
The retry() should not be needed, that was the purpose of screen:expect{any='%$'}. (retry() is like screen:expect() for non-screen tests).
If it was taking 13672 ms to complete, that implies that we might need the new unchanged option of screen:expect() added in #6930.
screen:expect{any='%$', unchanged=true}
But that seems wrong--there was no screen:expect() before this line, so it is not "unchanged" ...
janlazo
force-pushed
the
win_functional_tests
branch
from
265c9c0
to
e343b03
Compare
11 tasks
janlazo
force-pushed
the
win_functional_tests
branch
from
3171bfc
to
eba3b3b
Compare
|
@erw7 It worked. Why did the test take 13s? |
janlazo
force-pushed
the
win_functional_tests
branch
from
2e5d80d
to
2540f1a
Compare
janlazo
force-pushed
the
win_functional_tests
branch
3 times, most recently
from
dce344d
to
db9b690
Compare
janlazo
commented
Oct 23, 2018
| @@ -538,10 +538,6 @@ describe('API: buffer events:', function() | |||
| end) | |||
|
|
|||
| it('works with :diffput and :diffget', function() | |||
| if os.getenv("APPVEYOR") then | |||
There was a problem hiding this comment.
Passes on MSVC build. Why was it disabled on Appveyor?
janlazo
force-pushed
the
win_functional_tests
branch
4 times, most recently
from
96be38a
to
e973049
Compare
janlazo
changed the title
|
I'm done with the functional tests so I'm porting the remaining adjustments from #8405 for |
@janlazo see #8005 (#8156 may also be related). Did that somehow get fixed?
Is this PR in a "ready" state? Let's merge it before the big build.ps1 changes ... |
@justinmk Git's
It's "ready" but it has to be merged to master to test if it works with the cache and |
justinmk
reviewed
Oct 31, 2018
| $cmakeGeneratorArgs = '-v' | ||
| $mingwPackages = @('ninja', 'gcc', 'make', 'cmake', 'perl', 'diffutils', 'unibilium').ForEach({ | ||
| "mingw-w64-$arch-$_" | ||
| }) |
There was a problem hiding this comment.
Did you add gcc to upgrade gcc? If possible we should use whatever appveyor already has, otherwise it adds variability to the builds. Same with make. Also saves time.
There was a problem hiding this comment.
I added it in the event I run this script on Windows like #8405 but it's better to update Makefile for Windows.
justinmk
reviewed
Oct 31, 2018
|
|
||
| # Add MinGW to the PATH | ||
| $env:PATH = "C:\msys64\mingw$bits\bin;$env:PATH" | ||
|
|
||
| # Build third-party dependencies | ||
| C:\msys64\usr\bin\bash -lc "pacman --verbose --noconfirm -Su" ; exitIfFailed | ||
| C:\msys64\usr\bin\bash -lc "pacman --verbose --noconfirm --needed -S mingw-w64-$arch-cmake mingw-w64-$arch-perl mingw-w64-$arch-diffutils mingw-w64-$arch-unibilium" ; exitIfFailed | ||
| C:\msys64\usr\bin\bash -lc "pacman --verbose --noconfirm --needed -S $mingwPackages" ; exitIfFailed |
There was a problem hiding this comment.
I wonder if the build logs would be much shorter without --verbose ...
justinmk
reviewed
Oct 31, 2018
| cmd /c npm link neovim | ||
| npm.cmd install -g neovim ; exitIfFailed | ||
| Get-Command -CommandType Application neovim-node-host.cmd | ||
| npm.cmd link neovim |
There was a problem hiding this comment.
do we need ; exitIfFailed here?
There was a problem hiding this comment.
Not really. Keeping exitIfFailed stops the script at the npm.cmd command but the build will stop at Get-Command.
Close gzip file handles.
Give variables a default value to pass strict mode. $ErrorActionPreference defines the default behavior if a powershell command fails. If it's set to 'stop', then it aborts the script on the first unresolved error. This behavior extends to native programs like cmake but do not depend on it. PowerShell/PowerShell#3996
janlazo
force-pushed
the
win_functional_tests
branch
from
51808e9
to
8ce3d01
Compare
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.
#I'm using this PR to test assumptions for my vimrc and Appveyor environment. This will be similar to #7412 so expect frequent rebasing. I don't expect this to be merged for the 0.3 release.
Enable the following:
shelltemp,shellpipe, and related autocmd events and features (ie+quickfix).Thoughts/Wishlist for future releases (not included in this PR):
shellescapethat supports powershellshellto determine how the command token is escaped or ommit to use&shellfzf#shellescape(https://github.com/junegunn/fzf) or my version in https://github.com/janlazo/dotvim8/blob/master/autoload/dotvim8.vimshellslashwhich affects other functions (ie.fnamemodify) and discourage plugins authors to reinvent theirshellescapewhich is likely for cmd.exe only and doesn't consider edge cases