Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

chore(jasmine): Upgrade to jasmine 3.1.0 #4842

Closed
wants to merge 1 commit into from

Conversation

awarecan
Copy link

@awarecan awarecan commented Jun 4, 2018

Resolve #4788, #4708

Jamine 3 change the random default to true, however I want to keep protractor's default to false to avoid breaking changes.
done() accept error as parameter in Jasmine 3
beforeEach() no longer allow in it() in Jasmine 3

This is a rework for PR #4809

@awarecan
Copy link
Author

awarecan commented Jun 5, 2018

@qiyigg Could you restart the travis build? It passed in my fork repo, failed here due the slowness of travis I guess.

@qiyigg
Copy link
Contributor

qiyigg commented Jun 5, 2018

restarted.

@qiyigg
Copy link
Contributor

qiyigg commented Jun 6, 2018

Please take a look at the commit format and change the commit message accordingly:
https://github.com/angular/protractor/blob/master/CONTRIBUTING.md#commit-messages

This change makes sense to me, but updating major version of Jasmine involves with some breaking changes, I'd like to wait for a while.

@simonua
Copy link
Contributor

simonua commented Jun 6, 2018

@qiyigg, is there sense releasing a -beta package, not tagging it with latest?

@qiyigg
Copy link
Contributor

qiyigg commented Jun 6, 2018

@simonua One problem is do we keep merging new PR to beta? who maintains the beta? Essentially I don't want to let this repo to be too complicated, as you can see, I am the only person who takes care of this stuff in my free time. It would be great if more active contributors like you guys could help me maintain Protractor together.

@awarecan awarecan force-pushed the jasmine-3.1-rework branch from 0a15e09 to 316a6c5 Compare June 6, 2018 19:00
@simonua
Copy link
Contributor

simonua commented Jun 6, 2018

@qiyigg, I can understand that. On that note, thank you very much for your time and stewardship. I'm very surprised to hear you are the only active maintainer at this time. Scary, considering individuals and organizations rely on this package.

I don't mind being added as a collaborator, but I will readily admit that my in-depth knowledge of protractor is not where it may need to be to make decisions on its future or on PRs being merged.

@awarecan
Copy link
Author

awarecan commented Jun 6, 2018

Commit message updated.

No need for beta package. I can use it and convince other developers to use it in our real world projects as long as the PR merged, We can hold off publish npm release for a while, until more users tried the Jamine 3.

@awarecan
Copy link
Author

awarecan commented Jun 6, 2018

@qiyigg that confirmed my guessing. Haven't seen commits from @juliemr or @sjelin for a while.

@awarecan awarecan changed the title Upgrade to jasmine 3.1.0 (rework) chroe(jasmine): Upgrade to jasmine 3.1.0 Jun 6, 2018
@qiyigg qiyigg changed the title chroe(jasmine): Upgrade to jasmine 3.1.0 chore(jasmine): Upgrade to jasmine 3.1.0 Jun 6, 2018
@qiyigg
Copy link
Contributor

qiyigg commented Jun 6, 2018

I will keep this PR unmerged for a while and internally we are also trying to update this as well and it is not as easy as fixing Protractor itself. I need collect more information of how hard it is and we also need provide some tips for how to fix breaking tests for our users.

@simonua
Copy link
Contributor

simonua commented Jun 6, 2018

@awarecan, commit message: chroe -> chore

Update @types/jasmine to 2.8.8 as well
@awarecan awarecan force-pushed the jasmine-3.1-rework branch from 316a6c5 to 0ff487b Compare June 6, 2018 19:36
@awarecan
Copy link
Author

awarecan commented Jun 6, 2018

Some of my understanding for the break changes

Replace old "catch exceptions" logic with proper fail fast with error reporting

A new command line parameter -fail-fast=true provided, it also possible passed in jasmineNodeOpts.failFast in jasmine/jasmine-npm.
Maybe we should provide a wrapper and unit test for this option. (@qiyigg let me know if you guys will work on it or I can spend some time on it)

Detect an Error passed to done and add an expectation failure

The change like that need to be done if you use similar structure to test exception thrown in your test.
Regular positive test won't be affect.
image

Unify status for xdescribe and xit

It more likely will effect jasmine-reporters

Suite level errors all report the same way (on suiteDone)

It more likely will effect jasmine-reporters

Refactor QueueRunner and remove references to functions that Jasmine is done with

Don't know protractor ever use this QueueRunner

expect(null).toEqual(jasmine.any(Object)) no longer passes

That may affect particular usage

Default to running tests in random order

I handled that in my PR is to keep protractor still running test sequential unless you set jasmineNodeOpts.random = true

Additionally, Jasmine 3.0 drops support for older browsers and environments. Notably:

Not related with protractor

In general, I feel those changes are not so breaking, we can add a section to docs/jasmine-upgrade.md for better guide users

@asvishnyakov
Copy link

@awarecan Any update on this?

@Maximaximum
Copy link

Are there any updates on this very important PR? :)

@jscharett
Copy link

Can this PR be merged to add support for jasmine 3.x in protractor 5.x?

@awarecan
Copy link
Author

Protractor 6 already included jasmine 3.3, therefore I would like to close this PR.

@jscharett I don't think they will maintenance 5.x branch, fell free to take my code to change your local protractor package.

@awarecan awarecan closed this Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to jasmine 3.1.0
7 participants