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

Simplify test script pipeline #2851

Merged
merged 8 commits into from
Oct 27, 2017
Merged

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Sep 23, 2017

Description:
This PR fixes #2850, also revises current build pipeline around unit tests. Our test setup is quite complex, by using compiled output from src for importing modules, then build each (src, spec) separately to run test cases. This also causes coverage remapping tricky, as recent change caused.

Instead, PR changes test cases to require sources directly - then run compile just once for specs and create coverage as well. It get rid of few npm scripts for testing purpose only, performance wise doesn't have much differences.

Few caveat is it requires to update some other scripts like doc generation but it can be dealt with seperately.

Related issue (if exists):

@rxjs-bot
Copy link

rxjs-bot commented Sep 23, 2017

Warnings
⚠️

❗ Big PR (1)

Messages
📖

(1) : Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.

CJS: 1346.1KB, global: 746.2KB (gzipped: 120.0KB), min: 145.7KB (gzipped: 31.2KB)

Generated by 🚫 dangerJS

@kwonoj kwonoj force-pushed the chore-simplify-test branch 4 times, most recently from 04bc96a to da5105b Compare September 23, 2017 07:01
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.406% when pulling 18d3e59 on kwonoj:chore-simplify-test into 2e576dc on ReactiveX:master.

@kwonoj kwonoj requested a review from benlesh September 23, 2017 07:23
@kwonoj
Copy link
Member Author

kwonoj commented Sep 23, 2017

@benlesh for reviewing. Mostly it doesn't touch functionality but also it touches huge number of files.

@benlesh
Copy link
Member

benlesh commented Sep 26, 2017

This looks great OJ, I'll try to pull this down and run it locally before I merge it very soon.

@kwonoj kwonoj force-pushed the chore-simplify-test branch from 18d3e59 to 6564101 Compare September 27, 2017 17:53
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.07%) to 93.408% when pulling 6564101 on kwonoj:chore-simplify-test into c674581 on ReactiveX:master.

@benlesh
Copy link
Member

benlesh commented Oct 4, 2017

Apologies, @kwonoj, I dropped the ball on looking at this. We might want to wait until the 5.5 version is released and then come back to this, as the packaging/building scripts are in a little bit of flux right now.

@kwonoj
Copy link
Member Author

kwonoj commented Oct 4, 2017

I'm fine with that, feel freely to do so.

@benlesh
Copy link
Member

benlesh commented Oct 25, 2017

Okay... so this looks okay... however a 4% drop in code coverage is HUGE. We should look into that before proceeding.

@kwonoj kwonoj force-pushed the chore-simplify-test branch 2 times, most recently from 4501f52 to 3fb35c5 Compare October 25, 2017 18:46
@kwonoj kwonoj force-pushed the chore-simplify-test branch 4 times, most recently from 42006bd to 2a29515 Compare October 25, 2017 20:55
@kwonoj kwonoj force-pushed the chore-simplify-test branch from 2a29515 to be20525 Compare October 25, 2017 21:21
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 97.372% when pulling be20525 on kwonoj:chore-simplify-test into 24854cf on ReactiveX:master.

@kwonoj kwonoj force-pushed the chore-simplify-test branch 2 times, most recently from 721f8eb to 7443a88 Compare October 25, 2017 21:53
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 97.372% when pulling 7443a88 on kwonoj:chore-simplify-test into 24854cf on ReactiveX:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 97.372% when pulling 7443a88 on kwonoj:chore-simplify-test into 24854cf on ReactiveX:master.

@kwonoj kwonoj force-pushed the chore-simplify-test branch 2 times, most recently from a4fe70d to 2e29e1e Compare October 25, 2017 23:05
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 97.372% when pulling 2e29e1e on kwonoj:chore-simplify-test into 24854cf on ReactiveX:master.

@kwonoj kwonoj force-pushed the chore-simplify-test branch from 2e29e1e to 8d137ac Compare October 25, 2017 23:17
@coveralls
Copy link

coveralls commented Oct 25, 2017

Coverage Status

Coverage decreased (-0.03%) to 97.372% when pulling be20525 on kwonoj:chore-simplify-test into 24854cf on ReactiveX:master.

@kwonoj
Copy link
Member Author

kwonoj commented Oct 25, 2017

@benlesh it's good now, it was just glob wasn't smart enough to pick all test files. coverage change is within the threshold.

@kwonoj kwonoj merged commit 040d951 into ReactiveX:master Oct 27, 2017
@kwonoj kwonoj deleted the chore-simplify-test branch October 27, 2017 03:21
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
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.

Coverage report checker's broken
4 participants