Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt Electron 1.7.x #28582

Closed
bpasero opened this issue Jun 13, 2017 · 25 comments · Fixed by #31942 or #32857
Closed

Adopt Electron 1.7.x #28582

bpasero opened this issue Jun 13, 2017 · 25 comments · Fixed by #31942 or #32857
Assignees
Labels
on-testplan plan-item VS Code - planned item for upcoming
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Jun 13, 2017

Currently blocked by: electron/electron#9696

@bpasero bpasero added the plan-item VS Code - planned item for upcoming label Jun 13, 2017
@bpasero bpasero added this to the June 2017 milestone Jun 13, 2017
@bpasero bpasero self-assigned this Jun 13, 2017
@bpasero bpasero modified the milestones: June 2017, July 2017 Jun 14, 2017
@Tyriar
Copy link
Member

Tyriar commented Jul 17, 2017

@Tyriar
Copy link
Member

Tyriar commented Jul 17, 2017

The --build arg is causing the issue on Windows. This:

.\scripts\test.bat --build --reporter dot

Calls %CODE% .\test\electron\index.js %* which passes --build to test\electron\index.js.

@Tyriar
Copy link
Member

Tyriar commented Jul 18, 2017

It may not work because out-build doesn't exist. I can repro when doing:

npm run watch
.\scripts\test.bat --build --reporter dot

@Tyriar
Copy link
Member

Tyriar commented Jul 18, 2017

After spending quite a bit of time looking into this, I'm pretty stumped on why this is failing. @bpasero @joaomoreno any help would be great as I'm not sure where to go from here.

Here's what I've found so far:

  • I can reproduce using the following:

    git clean -xfd
    scripts\npm.bat i
    npm run gulp -- --max_old_space_size=4096 "vscode-win32-ia32-min"
    scripts\test.bat --build --reporter dot
    
  • Unit tests on Windows fail at random times

  • Passing --debug shows the window and all tests complete, with KeyboardMapper tests failing which I frequently when running unit tests on Windows.

    image

  • Listening to the IPCRunner.emit in addition to printing test names seems to stop at random spots, telling me that this isn't related to a single test. Here are the end of some logs from test runs:

    emit test end
    test start Types isArray
    emit test
    emit pass
    test end Types isArray
    emit test end
    test start Types isString
    emit test
    emit pass
    test end Types isString
    
    test start EventEmitter add 1 listener, remove it, emit event
    emit test
    emit hook
    emit hook end
    emit pass
    test end EventEmitter add 1 listener, remove it, emit event
    emit test end
    emit hook
    emit hook end
    
    emit start
    emit suite
    emit suite
    test start Browsers all
    emit test
    emit pass
    test end Browsers all
    emit test end
    emit suite end
    emit suite
    
  • The test runner pauses for 2-5 seconds after a the last output and then exits.

@bpasero
Copy link
Member Author

bpasero commented Jul 18, 2017

@Tyriar I just tried to reproduce on Windows, for me the tests are looking all OK. Do you see the test failures on the command line as output?

@Tyriar
Copy link
Member

Tyriar commented Jul 18, 2017

@bpasero the output stops before any failures example output:


(nothing, just line breaks)


......

............

etc.

@bpasero
Copy link
Member Author

bpasero commented Jul 19, 2017

@Tyriar does this reproduce with mocha tests (and the exact same node.js version that electron 1.7.4 uses) too?

@Tyriar
Copy link
Member

Tyriar commented Jul 20, 2017

Seems to happen when using node 7.9.0 as well which ships with 1.7.4. Currently the build machine is set up to use 7.10.0 (set in node.ps1).

@bpasero
Copy link
Member Author

bpasero commented Jul 20, 2017

Ok so it seems electron unrelated then.

Passing --debug shows the window and all tests complete, with KeyboardMapper tests failing which I frequently when running unit tests on Windows.

Can you explain this, are you saying the keyboard mapper test is causing this?

@Tyriar
Copy link
Member

Tyriar commented Jul 20, 2017

No, the keyboard mapper tests on Windows just have always seemed to fail for me a lot on most of my Windows machines.

@Tyriar
Copy link
Member

Tyriar commented Jul 24, 2017

Created electron/electron#10107 to track upstream.

It appears to be happening only when using IPCReporter because ipcRenderer.send calls stop making it to the main process after a short period.

@Tyriar
Copy link
Member

Tyriar commented Jul 25, 2017

Possibly related: nodejs/node#7657

@Tyriar
Copy link
Member

Tyriar commented Jul 26, 2017

Update:

It appears electron/electron#10107 is an issue with my home desktop only and is different to what we're seeing on the VSTS build server.

So I started to add some logs and run more builds on VSTS to see what is going on and it appears like the tests are exiting with app.exit(0) but %errorlevel% is being set to 255. I made a change to the test script to change error code 255 to 0 to get around this issue since the tests were passing anyway 7bed98e

After doing this I did a full build and it succeeded, however it does not launch. running with --verbose gives this:

C:\Users\Daniel\Downloads\VSCode-win32-x64-1.15.0-insider>"Code - Insiders.exe" --verbose

C:\Users\Daniel\Downloads\VSCode-win32-x64-1.15.0-insider>


#
# Fatal error in c:\e\workspace\libcc-win-x64\src\v8\src\snapshot\snapshot.h, line 28
# Check failed: IsSane().
#

==== C stack trace ===============================

        v8_inspector::protocol::Debugger::API::SearchMatch::fromJSONString [0x00007FFD149D145B+178411]
        V8_Fatal [0x00007FFD146B039D+125]
        v8::internal::NativesCollection<0>::GetScriptsSource [0x00007FFD14892246+9590]
        v8::Isolate::New [0x00007FFD14646EE6+326]
        GetHandleVerifier [0x00007FF7D3E2A602+1022466]
        GetHandleVerifier [0x00007FF7D3E2A4DC+1022172]
        GetHandleVerifier [0x00007FF7D3E2A495+1022101]
        (No symbol) [0x00007FF7D3AAE004]
        (No symbol) [0x00007FF7D3AE14AE]
        IsSandboxedProcess [0x00007FF7D41716A6+977938]
        IsSandboxedProcess [0x00007FF7D44338A5+3870225]
        IsSandboxedProcess [0x00007FF7D4416815+3751297]
        GetHandleVerifier [0x00007FF7D4081A09+3477001]
        GetHandleVerifier [0x00007FF7D4081900+3476736]
        GetHandleVerifier [0x00007FF7D3E2A32C+1021740]
        (No symbol) [0x00007FF7D3A87E5B]
        IsSandboxedProcess [0x00007FF7D6CFB0BB+46630439]
        BaseThreadInitThunk [0x00007FFD57652FA4+20]
        RtlUserThreadStart [0x00007FFD57E4CFA1+33]

@Tyriar
Copy link
Member

Tyriar commented Jul 26, 2017

That's pointing at this file, perhaps it's related to the snapshot stuff? @jrieken #28492

@Tyriar
Copy link
Member

Tyriar commented Jul 26, 2017

Commenting out the "Create loader snapshot" step from 1_build.ps1 fixes the exception on launch e65426f. Also integration tests are not running on VSTS for Windows, this may have caught the snapshot issue earlier.

@jrieken
Copy link
Member

jrieken commented Jul 26, 2017

@Tyriar When updating electron you must also bump the version of electron-snapshot. Unsure if there is electron migration guide that should have been updated...

@Tyriar
Copy link
Member

Tyriar commented Jul 27, 2017

Added to the guide 👍

@egamma egamma modified the milestones: August 2017, July 2017 Jul 28, 2017
@egamma
Copy link
Member

egamma commented Jul 28, 2017

Discussed with @kieferrm we will shift this to August.

@Tyriar
Copy link
Member

Tyriar commented Aug 2, 2017

@joaomoreno could you look into the exit code issue when you get a chance? We're seeing VSTS giving an exit code of 255 even though app.exit(0) appears to be getting called. The change for that is here: https://github.com/Microsoft/vscode/pull/31942/files#diff-81f83972da9328e55444850e3b36091e

@joaomoreno
Copy link
Member

When I log in to the build machine and try to run scripts\test.bat, the unit tests don't even run at all. All I see is two Code.exe processes stuck. Do you see this too?

image

I also can't find the build where you had a 255 exit code.

@bpasero
Copy link
Member Author

bpasero commented Aug 12, 2017

@Tyriar @roblourens interestingly I just noticed that Electron marks 1.7.5 as "latest release" while 1.7.4 is still a pre-release. We should consider being on the latest official release if possible imho.

image

image

@Tyriar
Copy link
Member

Tyriar commented Aug 14, 2017

@joaomoreno well I've never been able to log into the machine, I added an echo %errorlevel% and it was printing 255 after the tests. It looks like VSTS has discarded the build as it wasn't marked as retain. If the Windows builds succeed after this line is removed then all is well:

https://github.com/Microsoft/vscode/blob/1531c8d2f4610afb6f8575bc78d1083a5cb478dd/scripts/test.bat#L32

@Tyriar
Copy link
Member

Tyriar commented Aug 17, 2017

Update: I have Electron 1.7.5 on the branch tyriar/electron-1-7-5, I'm running into build issues on VSTS/macOS though which appear to be related to using the new file watcher in the build?

2017-08-17T02:32:40.3193210Z [04:32:40] [i18n] No localized message found for key relativeIconUrlRequiresHttpsRepository
2017-08-17T02:32:40.3205310Z [04:32:40] [i18n] No localized message found for key relativeBadgeUrlRequiresHttpsRepository
2017-08-17T02:32:40.3217850Z events.js:163
2017-08-17T02:32:40.3234730Z       throw er; // Unhandled 'error' event
2017-08-17T02:32:40.3251300Z       ^
2017-08-17T02:32:40.3256600Z 
2017-08-17T02:32:40.3274350Z Error: ENFILE: file table overflow, open '/Users/code/tfs/agent3/_work/2/s/extensions/json/server/node_modules/ms/readme.md'
2017-08-17T02:32:40.3644680Z 
2017-08-17T02:32:40.3724250Z npm ERR! Darwin 15.3.0
2017-08-17T02:32:40.3742940Z npm ERR! argv "/Users/code/.nvm/versions/node/v7.10.0/bin/node" "/Users/code/.nvm/versions/node/v7.10.0/bin/npm" "run" "gulp" "--" "vscode-darwin-min" "upload-vscode-sourcemaps"
2017-08-17T02:32:40.3763770Z npm ERR! node v7.10.0
2017-08-17T02:32:40.3775140Z npm ERR! npm  v4.2.0
2017-08-17T02:32:40.3786550Z npm ERR! code ELIFECYCLE
2017-08-17T02:32:40.3798160Z npm ERR! errno 1
2017-08-17T02:32:40.3816040Z npm ERR! code-oss-dev@1.16.0 gulp: `gulp --max_old_space_size=4096 "vscode-darwin-min" "upload-vscode-sourcemaps"`
2017-08-17T02:32:40.3835260Z npm ERR! Exit status 1
2017-08-17T02:32:40.3847860Z npm ERR! 
2017-08-17T02:32:40.3867020Z npm ERR! Failed at the code-oss-dev@1.16.0 gulp script 'gulp --max_old_space_size=4096 "vscode-darwin-min" "upload-vscode-sourcemaps"'.
2017-08-17T02:32:40.3955810Z npm ERR! Make sure you have the latest version of node.js and npm installed.
2017-08-17T02:32:40.3985600Z npm ERR! If you do, this is most likely a problem with the code-oss-dev package,
2017-08-17T02:32:40.4019720Z npm ERR! not with npm itself.
2017-08-17T02:32:40.4042080Z npm ERR! Tell the author that this fails on your system:
2017-08-17T02:32:40.4089040Z npm ERR!     gulp --max_old_space_size=4096 "vscode-darwin-min" "upload-vscode-sourcemaps"
2017-08-17T02:32:40.4136260Z npm ERR! You can get information on how to open an issue for this project with:
2017-08-17T02:32:40.4153810Z npm ERR!     npm bugs code-oss-dev
2017-08-17T02:32:40.4187170Z npm ERR! Or if that isn't available, you can get their info via:
2017-08-17T02:32:40.4218010Z npm ERR!     npm owner ls code-oss-dev
2017-08-17T02:32:40.4246120Z npm ERR! There is likely additional logging output above.
2017-08-17T02:32:40.4256410Z 
2017-08-17T02:32:40.4267660Z npm ERR! Please include the following file with any support request:
2017-08-17T02:32:40.4314550Z npm ERR!     /Users/code/tfs/agent3/_work/npm-cache/_logs/2017-08-17T02_32_40_412Z-debug.log

Not sure why an updated Electron would make this error occur yet though.

@Tyriar Tyriar reopened this Aug 17, 2017
@roblourens
Copy link
Member

@Tyriar We got that error in the official smoke tests and Andre said in Slack that the Mac build machine was messed up. Try it again.

@Tyriar
Copy link
Member

Tyriar commented Aug 17, 2017

Will do

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
on-testplan plan-item VS Code - planned item for upcoming
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants