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

Skip failing tests of macOS and add to CI #690

Merged
merged 6 commits into from
Jul 9, 2017

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Jun 9, 2017

Fixes #687

@Tyriar Tyriar added this to the 2.8.0 milestone Jun 9, 2017
@Tyriar Tyriar self-assigned this Jun 9, 2017
@Tyriar Tyriar requested a review from parisk June 9, 2017 23:28
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 69.489% when pulling 6480933 on Tyriar:687_macos_ci into a889fef on sourcelair:master.

@Tyriar Tyriar added the work-in-progress Do not merge label Jun 12, 2017
@parisk
Copy link
Contributor

parisk commented Jun 15, 2017

Seems to be crashing at "install" time because of node-gyp.

@Tyriar
Copy link
Member Author

Tyriar commented Jun 15, 2017

Yeah something related to compilation of the sleep module. Not sure how to fix it right now.

Might be easier to look for alternatives here https://github.com/sourcelair/xterm.js/blob/795cdb52dc585ebdf5b47a93f50582e7ce954ad4/src/test/escape-sequences-test.js#L30

@Tyriar Tyriar removed this from the 2.8.0 milestone Jun 18, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 69.41% when pulling b520e2f on Tyriar:687_macos_ci into 2b92996 on sourcelair:master.

@coveralls
Copy link

coveralls commented Jul 9, 2017

Coverage Status

Coverage increased (+0.01%) to 69.43% when pulling 0536862 on Tyriar:687_macos_ci into 2b92996 on sourcelair:master.

@Tyriar Tyriar removed the work-in-progress Do not merge label Jul 9, 2017
@Tyriar Tyriar added this to the 2.9.0 milestone Jul 9, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 69.43% when pulling 5e815de on Tyriar:687_macos_ci into 2b92996 on sourcelair:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 69.43% when pulling 5e815de on Tyriar:687_macos_ci into 2b92996 on sourcelair:master.

Copy link
Contributor

@parisk parisk left a comment

Choose a reason for hiding this comment

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

Great! 🎉

@Tyriar Tyriar merged commit decd81b into xtermjs:master Jul 9, 2017
@Tyriar Tyriar deleted the 687_macos_ci branch July 9, 2017 08:55
bgw added a commit to bgw/xterm.js that referenced this pull request Mar 29, 2018
This is at least a first step towards fixing xtermjs#1247. This adds some
complexity to the initial setup, since canvas needs to be built from
source.

This won't be true once canvas 2.x is stabilized, because that version
downloads (Automattic/node-canvas#992). JSDom doesn't support 2.x, and
doesn't until a stable release is cut.

We could use
[canvas-prebuilt](https://github.com/node-gfx/node-canvas-prebuilt),
which JSDom does appear to support. However, I'm not sure if that would
cause pain for people with 32-bit operating systems (see the
compatibility table on that page).

I'm not sure if this travis configuration will work; I'll iterate on it
in the PR if it fails. I removed a bunch of hacks that were added
in xtermjs#690, since they don't look necessary anymore (the sleep module is
gone).
bgw added a commit to bgw/xterm.js that referenced this pull request Apr 18, 2018
This is at least a first step towards fixing xtermjs#1247. This adds some
complexity to the initial setup, since canvas needs to be built from
source.

This won't be true once canvas 2.x is stabilized, because that version
downloads (Automattic/node-canvas#992). JSDom doesn't support 2.x, and
doesn't plan to (jsdom/jsdom#1964) until a stable release is cut.

We could use
[canvas-prebuilt](https://github.com/node-gfx/node-canvas-prebuilt),
which JSDom does appear to support. However, I'm not sure if that would
cause pain for people with 32-bit operating systems (see the
compatibility table on that page).

I'm not sure if this travis configuration will work; I'll iterate on it
in the PR if it fails. I removed a bunch of hacks that were added
in xtermjs#690, since they don't look necessary anymore (the sleep module is
gone).
bgw added a commit to bgw/xterm.js that referenced this pull request Apr 18, 2018
This is at least a first step towards fixing xtermjs#1247. This adds some
complexity to the initial setup, since canvas needs to be built from
source.

This won't be true once canvas 2.x is stabilized, because that version
downloads (Automattic/node-canvas#992). JSDom doesn't support 2.x, and
doesn't plan to (jsdom/jsdom#1964) until a stable release is cut.

We could use
[canvas-prebuilt](https://github.com/node-gfx/node-canvas-prebuilt),
which JSDom does appear to support. However, I'm not sure if that would
cause pain for people with 32-bit operating systems (see the
compatibility table on that page).

I'm not sure if this travis configuration will work; I'll iterate on it
in the PR if it fails. I removed a bunch of hacks that were added
in xtermjs#690, since they don't look necessary anymore (the sleep module is
gone).
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