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

CI with Angular 8 #3220

Merged
merged 10 commits into from Jun 24, 2019
Merged

CI with Angular 8 #3220

merged 10 commits into from Jun 24, 2019

Conversation

maxokorokov
Copy link
Member

@maxokorokov maxokorokov commented Jun 3, 2019

The goal of this PR is to make CI pass with Angular 8 (without ivy)

  • Update dependencies: Angular 8.0, Bootstrap 4.3.1, latest CLI deps
  • add {static: true/false} for queries
  • fix flaky tests → execution order changed with latest dependencies, so some tests were leaving DOM nodes and some tests were using ids like popover-1 depending on test execution order
  • fix TS compilation issues

Please see separate commits and don't squash when merging

Fixes #3213

@maxokorokov maxokorokov added this to the 5.0 milestone Jun 3, 2019
@maxokorokov maxokorokov requested a review from benouat June 3, 2019 11:28
@maxokorokov maxokorokov modified the milestones: 5.0, 5.0.0-rc.0 Jun 3, 2019
@maxokorokov maxokorokov changed the title Chore: CI with Angular 8 CI with Angular 8 Jun 3, 2019
@maxokorokov

This comment has been minimized.

@maxokorokov
Copy link
Member Author

Well, there are definitely some positioning issues when running tests in parallel (exactly 18px on the Y axis, if you see logs).

  • I can't reproduce them locally even by running a couple of browsers in parallel
  • Might be related to updates in Karma/Jasmine/runners, but not sure

@codecov-io
Copy link

Codecov Report

Merging #3220 into master will decrease coverage by 1.89%.
The diff coverage is 95%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3220     +/-   ##
=========================================
- Coverage   92.04%   90.14%   -1.9%     
=========================================
  Files          91       91             
  Lines        3054     2700    -354     
  Branches      505      503      -2     
=========================================
- Hits         2811     2434    -377     
- Misses        179      207     +28     
+ Partials       64       59      -5
Flag Coverage Δ
#e2e 55.03% <80%> (-7.97%) ⬇️
#unit 87.28% <95%> (-2.2%) ⬇️
Impacted Files Coverage Δ
src/util/autoclose.ts 84% <0%> (ø) ⬆️
src/rating/rating.ts 96.92% <100%> (-0.38%) ⬇️
src/accordion/accordion.ts 98.43% <100%> (-0.3%) ⬇️
src/datepicker/datepicker.ts 97.75% <100%> (-1.25%) ⬇️
src/pagination/pagination.ts 100% <100%> (ø) ⬆️
src/dropdown/dropdown.ts 93.79% <100%> (-0.82%) ⬇️
src/popover/popover.module.ts 50% <0%> (-50%) ⬇️
src/modal/modal.module.ts 50% <0%> (-50%) ⬇️
src/carousel/carousel.module.ts 50% <0%> (-50%) ⬇️
src/collapse/collapse.module.ts 50% <0%> (-50%) ⬇️
... and 79 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a17683...19a138f. Read the comment docs.

Copy link
Member

@benouat benouat left a comment

Choose a reason for hiding this comment

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

LGTM! I updated some stuff via the latest 2 commits:

  • fallbacking the es5 target for testing in IE (mostly duplicated some configs files for Saucelabs)
  • making the build green with positioning tests (needed to add bootstrap styles to karma)

@benouat benouat mentioned this pull request Jun 14, 2019
5 tasks
@benouat benouat merged commit a20df01 into ng-bootstrap:master Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update for angular 8 support
3 participants