Skip to content

Testing Components with TestBed #1175

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

Merged
merged 27 commits into from
May 16, 2018
Merged

Conversation

hypery2k
Copy link
Contributor

See #1061, rebased to latest master

justindujardin and others added 4 commits December 24, 2017 09:58
 - update zone build and add jasmine/mocha test scripts, see: NativeScript/zone.js#1
 - refactor testing and zone-js modules to work with each other
 - add generous TestBed setup helpers in testing/src/util.ts
 - add single import test helpers for zone-js patches in zone-js/testing.[framework].ts files
 - update peer deps to reflect updated zone (could peer dep be dropped since prebuilts are exported?)

chore: disable most tests and enable ones that use TestApp one-by-one
 - add test entry point script that inits the test environment for TestBed
 - list view, modal dialog pass
 - detached-loader and platform-filter-component could use feedback. see todos

chore: replace the remaining TestApp usages in test suite

 - xdescribe the failing tests.
 - I think the remaining problems boil down to `dumpView` indicating the ComponentFixture comes back with the root components, and `@ViewChild` not finding DetachedLoader by its class.
 - remove some duplication in testing utilities

chore: cleanup and remove some diff noise from a few tests

chore: remove more test noise

 - are the line-endings different on this file? :(

chore: convince dumpView and TestComponentRenderer to agree on things

 - all the TestBed tests except for the DetachedLoader ones and a single Renderer lifecycle are passing.
 - update NativeScriptRenderer.selectRootElement to find views by ID when given a selector of an id string. TestBed uses this when creating componentRefs to get at the correct views.
 - change NativeScriptTestComponentRenderer to inject only a ProxyViewContainer which mimics what TestApp did.
 - update dumpView to strip off the new "source" data attached to a view.toString() result.

chore: cleanup lint

chore: make nTestBed helpers automatically clean up test components

 - before the components were destroyed by TestBed, but not removed from the rootView.
 - maintain a list of active fixtures for a set of tests, and remove them all when the tests complete.
 - reorder to the testing utils to flow better when reading (start with test init, then before/after then render components)
 - clean up some lint.

chore: fix issue where nTestBedBeforeEach overwrote its own imports

 - Fixes the DetachedLoader tests, and makes them MUCH simpler.
 - When you configure the test bed module, you need to specify a full list of imports, because they completely overwrite the imports array that is used.
 - That's yet another reason to use the provided helper functions, they merge in the common {N} imports for you.
 - ... and some lint cleanup

chore: make renderer lifecyle test more robust

 - the first assertion is that the view after init has been called. rather than assert it, just wait for it using an observable and avoid asserting about timing and implementation specific details of the system. Specifically this removes the assumption that `app.tick()` will advance time and call `ngAfterViewInit` on the component.

chore: cleanup from review

 - re-enable all tests in karma.conf.js
 - remove some diff noise
 - incorporate changes from zone to include @panayot.cankov's drainMicroTaskQueue update
 - update zone.js deps to reference "*" for version. Ideally it would be removed, but I believe @angular has a peer dependency on it so satisfy it with a star. This is because zone.js files are prebuilt specifically for {N} so there is no need to bring them in as a dep.
 - the TestBed changes do not depend on the upgrade (after review), and given that there are strange errors, prefer to reduce the number of changes that are not strictly required.
@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

9 similar comments
@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ns-bot
Copy link

ns-bot commented Jan 29, 2018

Can one of the admins verify this patch?

@ghost ghost added the ♥ community PR label Jan 29, 2018
@hypery2k hypery2k mentioned this pull request Feb 4, 2018
12 tasks
@hypery2k
Copy link
Contributor Author

hypery2k commented Feb 8, 2018

@sis0k0 Any chance to get it merged?

@hypery2k
Copy link
Contributor Author

@hdeshev can you run ci again?

@ns-bot
Copy link

ns-bot commented Feb 13, 2018

Can one of the admins verify this patch?

@sis0k0
Copy link
Contributor

sis0k0 commented Feb 13, 2018

run ci

@hypery2k
Copy link
Contributor Author

can you post the logs from the ios-renderer?

@ddfreiling
Copy link
Contributor

Is this going to be integrated? Would love to use in our current project.

@hypery2k
Copy link
Contributor Author

@sis0k0 Any chance to get it merged?

@hypery2k
Copy link
Contributor Author

@sis0k0 Can you rerun ci?

@hypery2k
Copy link
Contributor Author

@hdeshev can you run ci again?

vakrilov and others added 2 commits May 3, 2018 18:07
Copy link
Contributor Author

@hypery2k hypery2k left a comment

Choose a reason for hiding this comment

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

Already in your PR ;)

@hypery2k
Copy link
Contributor Author

hypery2k commented May 4, 2018

@vakrilov Looking forward for the Jenkins build ;)

@hypery2k
Copy link
Contributor Author

on my jenkins the tests are working so far with iOS and Android, see here.

After travis run is complete i will rebase

@hypery2k
Copy link
Contributor Author

@vakrilov I think your request for changes is blocking the CI run

@vakrilov
Copy link
Contributor

test

@hypery2k
Copy link
Contributor Author

travis seems having issues with the Github payload: https://travis-ci.org/NativeScript/nativescript-angular/requests

@SvetoslavTsenov
Copy link
Contributor

test

@SvetoslavTsenov
Copy link
Contributor

test

@hypery2k
Copy link
Contributor Author

At my jenkins the latest branch seems to work, see here. Can somebody post the results?

vakrilov and others added 2 commits May 15, 2018 16:48
fix(zone): Fix zone patching Promise prop descriptor and breaking a…
@vakrilov
Copy link
Contributor

test

@vakrilov vakrilov merged commit 52f3ec6 into NativeScript:master May 16, 2018
@ghost ghost removed the ♥ community PR label May 16, 2018
@vakrilov
Copy link
Contributor

Cheers @justindujardin and @hypery2k for the big effort!

@hypery2k
Copy link
Contributor Author

you're welcome. Can't wait to try it in my apps ;)

@hypery2k hypery2k deleted the testing branch May 16, 2018 15:36
@jlooper
Copy link

jlooper commented May 16, 2018

hi @justindujardin and @hypery2k ! This is an awesome PR and qualifies for a special badge!
contrib

If you'd like to email me your shirt sizes I'm happy to send you a tshirt and stickers (or alternately socks or hat - let me know what swag you'd like :)) jen.looper @ progress.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants