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

Electron 6 Test Failure: Search reports telemetry on search completed #80251

Closed
bpasero opened this issue Sep 3, 2019 · 2 comments · Fixed by microsoft/azuredatastudio#7206
Closed
Assignees
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Sep 3, 2019

It looks like recently there is a test failure that I can only reproduce from the electron-6.0.x branch:

1) SearchModel Search Model: Search reports telemetry on search completed:

      AssertionError [ERR_ASSERTION]: false == true
      + expected - actual

      -false
      +true
      
      at Context.<anonymous> (file:///Users/bpasero/Development/Microsoft/vscode-electron-6.0.x/out/vs/workbench/contrib/search/test/common/searchModel.test.js:110:20)
      at processTicksAndRejections (internal/process/task_queues.js:89:5)

Must have been a regression from not too recent as this is the first time I am seeing that failing.

@bpasero
Copy link
Member Author

bpasero commented Sep 4, 2019

I pushed e5db2c3 as workaround. My testing shows that everything seems to be working fine, however:

  • when await testObject.search({ contentPattern: { pattern: 'somestring' }, type: 1, folderQueries }); returns, the onDoneStopwatch(duration => this.telemetryService.publicLog('searchResultsFinished', { duration })); has not yet been fired. Assuming that the one happens before the other seems fishy to me.

Maybe something changed with how in E6 async code is ordered. This possibly came in when we switched to compile to ES7 from ES6 and native async await.

@roblourens roblourens added this to the September 2019 milestone Sep 4, 2019
@roblourens
Copy link
Member

Yeah that's not good async code, fixed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants