Skip to content
This repository was archived by the owner on May 25, 2019. It is now read-only.

"fix" PhantomJS test failures, depend on codemirror >3 #57

Merged
merged 2 commits into from
Jul 4, 2014

Conversation

whitehat101
Copy link
Contributor

Resolves #47

codemirror/codemirror5#2409

PhantomJS is starting to feel increasingly shaky. I don't really want to add too much code to the core to deal with its bugs. In our own test suite, tests that fail on Phantom and work on real browsers are simply disabled when running on Phantom. I suggest you do the same in your tests.

Since the maintainer of CodeMirror doesn't want to add bytes his javascript to get test that work in real browsers to pass to work in PhantomJS, we should do what he does and skip the failing tests as well.

I took his suggestion, and implemented the same solution he uses in his code.

I also changed the bower.json dependency for codemirror from "^3" to ">3".

@@ -22,7 +22,7 @@
},
"dependencies": {
"angular": "^1",
"codemirror": "^3"
"codemirror": ">3"
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to stay safe with major versions.
Like we don't know about v5.x.x and more, let's go with >=3 <=4 or >2 <5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'm looking up node's semver.

>=3 <=4 # won't allow 4.1.x
>2 <5   # will allow 2.1 
^3 || ^4
~3 || ~4
3 || 4

(I believe) the above are equivalent, and map to >=3.0.0-0 <5.0.0-0. I'll stick in 3 || 4 because it's concise and readable, imo.

@douglasduteil
Copy link
Contributor

This PR is cool @whitehat101
Can you rebase it on the current master version?
thanks

douglasduteil added a commit that referenced this pull request Jul 4, 2014
"fix" PhantomJS test failures, depend on codemirror >3
@douglasduteil douglasduteil merged commit 1adc840 into angular-ui:master Jul 4, 2014
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 this pull request may close these issues.

bump CodeMirror to 4.0.x
2 participants