Skip to content

build: update jasmine to 3.5#36501

Closed
JiaLiPassion wants to merge 1 commit intoangular:9.1.xfrom
JiaLiPassion:jasmine-3.5-9_1_x
Closed

build: update jasmine to 3.5#36501
JiaLiPassion wants to merge 1 commit intoangular:9.1.xfrom
JiaLiPassion:jasmine-3.5-9_1_x

Conversation

@JiaLiPassion
Copy link
Copy Markdown
Contributor

based on PR #34625, this one will target 9.1.x branch.

  1. update jasmine to 3.5
  2. update @types/jasmine to 3.5
  3. update @types/jasminewd2 to 2.0.8

Also fix several cases, the new jasmine 3 will help to create test cases correctly,
such as in the jasmine 2.x version, the following case will pass

expect(1 == 2);

But in jsamine 3, the case will need to be

expect(1 == 2).toBeTrue();

Some cases will still need to use spy as any cast, because @types/jasmine have some issues,

  1. The issue jasmine doesn't handle optional method properties, [@types/jasmine] Jasmine SpyOn type errors with optional methods DefinitelyTyped/DefinitelyTyped#43486
  2. The issue jasmine doesn't handle overload method correctly, @types/jasmine toHaveBeenCalledWith infers parameters for overloads incorrectly DefinitelyTyped/DefinitelyTyped#42455

@JiaLiPassion JiaLiPassion requested a review from IgorMinar April 8, 2020 08:26
@JiaLiPassion JiaLiPassion added area: build & ci Related the build and CI infrastructure of the project and removed cla: yes labels Apr 8, 2020
@ngbot ngbot Bot added this to the needsTriage milestone Apr 8, 2020
@googlebot
Copy link
Copy Markdown

☹️ Sorry, but only Googlers may change the label cla: yes.

@ngbot ngbot Bot added this to the needsTriage milestone Apr 8, 2020
@JiaLiPassion JiaLiPassion mentioned this pull request Apr 8, 2020
12 tasks
@JiaLiPassion JiaLiPassion force-pushed the jasmine-3.5-9_1_x branch 3 times, most recently from fe55d99 to 0fb79f8 Compare April 8, 2020 12:14
Copy link
Copy Markdown
Contributor

@ayazhafiz ayazhafiz left a comment

Choose a reason for hiding this comment

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

lgtm for language service

@pullapprove pullapprove Bot requested a review from alxhub April 10, 2020 02:15
1. update jasmine to 3.5
2. update @types/jasmine to 3.5
3. update @types/jasminewd2 to 2.0.8

Also fix several cases, the new jasmine 3 will help to create test cases correctly,
such as in the `jasmine 2.x` version, the following case will pass

```
expect(1 == 2);
```

But in jsamine 3, the case will need to be

```
expect(1 == 2).toBeTrue();
```

Some cases will still need to use `spy as any` cast, because `@types/jasmine` have some issues,
1. The issue jasmine doesn't handle optional method properties, DefinitelyTyped/DefinitelyTyped#43486
2. The issue jasmine doesn't handle overload method correctly, DefinitelyTyped/DefinitelyTyped#42455
@mhevery mhevery added the action: merge The PR is ready for merge by the caretaker label May 6, 2020
@mhevery
Copy link
Copy Markdown
Contributor

mhevery commented May 6, 2020

presubmit

Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

Hi @JiaLiPassion, thanks for the PR but I'm a bit puzzled to be honest...

This PR doesn't actually "update to jasmine 3.5" - can you please clarify what you are trying to do?

Comment thread package.json
"@types/hammerjs": "2.0.35",
"@types/inquirer": "^6.5.0",
"@types/jasmine": "3.5.10",
"@types/jasmine": "^3.5.10",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why this change? it seems unnecessary and it's actually undesirable.

we found it easier know what we use if we pin the dependencies in package.json and manually update. Can you please roll back this change (and yarn.lock file).

@JiaLiPassion
Copy link
Copy Markdown
Contributor Author

@IgorMinar , thank you for the review. This PR is based on

@JiaLiPassion
Copy link
Copy Markdown
Contributor Author

@IgorMinar , sorry for the confusion, it seems the PR #34625 has been merged into branch 9.1.x last week maybe, so this PR is no longer needed.

@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Jun 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants