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

Fix API tests on windows and add missing await on ts test #7655

Merged
merged 2 commits into from
Apr 23, 2020
Merged

Conversation

amiramw
Copy link
Member

@amiramw amiramw commented Apr 23, 2020

Signed-off-by: Amiram Wingarten amiram.wingarten@sap.com

What it does

Fixes #7644
Fixes API tests on windows and add missing await on typescript.spec.js.

How to test

Run yarn test:browser on windows and see that it completes successfuly.

Review checklist

Reminder for reviewers

Signed-off-by: Amiram Wingarten <amiram.wingarten@sap.com>
@amiramw amiramw requested a review from akosyakov April 23, 2020 06:42
@amiramw
Copy link
Member Author

amiramw commented Apr 23, 2020

@akosyakov is this missing await related to #7635 maybe?

@akosyakov
Copy link
Member

@amiramw you are my hero! sure it fixes #7635

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much ❤️

please merge if the build is green

@akosyakov akosyakov added ci issues related to CI / tests test issues related to unit and api tests labels Apr 23, 2020
@amiramw
Copy link
Member Author

amiramw commented Apr 23, 2020

Now travis fail on typescript.spec.js:

   editor.action.peekDefinition
     within an editor:

�[0m�[31m Error: Timeout of 45000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.�[0m�[90m

@akosyakov
Copy link
Member

@amiramw I can reproduce it locally. It seems to be sometimes dispatching esc button happens too early and Monaco does not handle it. Looking into it. I will push a commit to this PR ok?

I will reduce timeout back to 30s, i've increased it because could not understand flakiness, but it was missing await.

@amiramw
Copy link
Member Author

amiramw commented Apr 23, 2020

sure. thanks!

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
Copy link
Member

Pushed. There were a race in tests, that close happens before the peek widget was actually focused. Merging if the build is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests test issues related to unit and api tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API tests do not start on windows
2 participants