Skip to content

Test on PR and push to master #172

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

Merged
merged 1 commit into from
Aug 1, 2021
Merged

Test on PR and push to master #172

merged 1 commit into from
Aug 1, 2021

Conversation

brxck
Copy link
Collaborator

@brxck brxck commented Aug 1, 2021

This runs tests for linux, mac, and windows on every PR and push to master.

Tests occasionally flake out, it seems like adding a 100ms wait after decoration set up solves this. I've run the tests a few times successfully now. The timeout could be smaller, but things are pretty bad on the macos runner which is pretty dang slow. I'm guessing because of this:

Unable to create basic Accelerated OpenGL renderer.

Core Image is now using the software OpenGL renderer. This will be slow.

From what I've been able to read, it doesn't seem like we're going to be able to do much about that...?

@brxck brxck force-pushed the ci-test branch 13 times, most recently from f7bbbc8 to f6235ff Compare August 1, 2021 18:00
@brxck brxck marked this pull request as ready for review August 1, 2021 18:13
@brxck brxck requested a review from pokey as a code owner August 1, 2021 18:13
@pokey pokey merged commit cc7d4e7 into master Aug 1, 2021
@pokey pokey deleted the ci-test branch August 1, 2021 18:16
@@ -82,6 +82,7 @@ suite("recorded test cases", async function () {

// Wait for cursorless to set up decorations
cursorlessApi.addDecorations();
await new Promise((resolve) => setTimeout(resolve, 100));
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be necessary; let's figure out how to remove it in a follow-up

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Aug 2, 2021

@brxck Is it only mac that needs the sleep time? If that's the case we could probably do something like this. Not as optimal as removing the sleep totally foriall platforms but still better than having them where they are not needed.

if (os.platform() === "darwin") {
        await new Promise((resolve) => setTimeout(resolve, 100));
 }

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

Successfully merging this pull request may close these issues.

3 participants