Upgrade to [email protected] and use Jasmine2 #5646
Conversation
|
@zpao Ping. |
|
@zpao updated the pull request. |
zpao
force-pushed
the
upgrade-testing
branch
from
08ee0e0
to
f6f8b87
Compare
|
@zpao updated the pull request. |
|
@zpao Ping. Jasmine2 has much better async support, which I'd like to start using. Also, this PR is getting stale and will only get harder to merge as time goes on. |
|
@jimfb If you want to get this merged, do you want to figure out the metamatchers tests? |
|
You don't need to keep pinging me, I know this is on my plate. I'm also well versed in the difficulty of rebasing this, I've done it a couple times. The MetaMatchers thing is hard and requires a bunch of time that I haven't had. You are welcome to try to fix it yourself. |
|
It had been a couple months with no updates, which is fine, but we have a bunch of these open/lingering PRs, so I just wanted to bring it to your attention in case it had been forgotten. Anyway, with regards to MetaMetchers - I agree it looks like some nasty jasmine logic in there. The MetaMatchers is only used in one test, which is testing tests rather than testing React. I agree it's useful/valuable, but I wonder if it is worth the weight, especially if it's the only thing blocking an update. It might (in the interest of making forward progress) make more sense to just disable that test and get this merged. We could always re-enable it later when someone has the time to make it work. Anyway, just an idea, your call either way. |
|
As evidenced by my lack of finishing this, it's highly unlikely we ever come back to re-enable it. I think the test is pretty important to ensure we have the same set of expectations across TS, JS, CS classes. |
|
Superseded by #6769 |
|
Thanks for the reminder! |
There is 1 lingering issue that I haven't had a chance to look too much into and that's the MetaMatchers. It does some hacky stuff Jasmine and at first glance doesn't appear to be compatible with Jasmine 2. I'll take a closer look at it shortly.