Test/add specs #85

bfricka · 2017-07-16T07:07:17Z

A few things to note about this PR:

initialState function for features. This was only implemented for root previously, but I added it to features as well. Unfortunately, the implementation is a bit different b/c features aren't using a token to provide initial state.
Removed OpaqueToken. The deps for this project are clear, and we're already using InjectionToken elsewhere, so it doesn't make sense to continue using a deprecated symbol.
Stronger types on core actions INIT, UPDATE, etc. I don't think this actually affects anything currently, but it does make certain inspections nicer.
Watch mode using chokidar. This should make writing development a bit nicer (yarn run watch:tests). This also outputs an html coverage report if that's your cup.

Some of these tests are a bit obtuse. I'm still getting used to writing ng2+ tests, so feedback is welcome if you see something that could be improved. I'll write some more tests in the future, but I wanted to get this out there.

ps: Crossing my fingers that the xit test passes. Last time it failed in CI, and I have zero clue why. If it fails I'll xit back again

coveralls · 2017-07-16T07:13:29Z

Coverage increased (+1.5%) to 90.029% when pulling bb489e8 on bfricka:test/add-specs into b9776ea on ngrx:master.

- Add watch:tests for simpler dev / test - Store utils spec

bfricka · 2017-07-18T22:14:18Z

Hey there! First off, congrats on 4.0.0!

Second, I resolved conflicts!

coveralls · 2017-07-18T22:16:36Z

Coverage increased (+1.4%) to 89.81% when pulling 7f59f72 on bfricka:test/add-specs into a5aad22 on ngrx:master.

brandonroberts · 2017-07-19T01:11:13Z

modules/store/spec/store.spec.ts

      const result = store.select('counter1');

-      expect(result instanceof Store).toBe(true);
+      expect(result).toEqual(any(Store));


Using instanceof seems like a better check than any(Store)

brandonroberts · 2017-07-19T01:11:32Z

modules/store/spec/store.spec.ts

@@ -183,7 +177,7 @@ describe('ngRx Store', () => {
    });

    // TODO: Investigate why this is no longer working
-    xit('should complete if the dispatcher is destroyed', () => {


Let this test remain excluded

brandonroberts · 2017-07-19T01:12:19Z

modules/store/spec/utils.spec.ts

@@ -0,0 +1,81 @@
+/**


Remove header text

brandonroberts · 2017-07-19T01:16:03Z

modules/store/src/store_module.ts

@@ -51,7 +51,13 @@ export class StoreFeatureModule implements OnDestroy {
    @Inject(STORE_FEATURES) private features: StoreFeature<any, any>[],
    private reducerManager: ReducerManager
  ) {
-    features.forEach(feature => reducerManager.addFeature(feature));
+    for (let feature of features) {


The functional approach is preferred over the for loop.

You got it!

brandonroberts · 2017-07-19T01:16:49Z

modules/store/src/utils.ts

-  return Object.keys(object)
-    .filter(key => key !== keyToRemove)
-    .reduce((result, key) => Object.assign(result, { [key]: object[key] }), {});
+  const ret: Partial<T> = {};


No need to refactor this.

brandonroberts · 2017-07-19T01:19:17Z

modules/store/src/state.ts

 import { withLatestFrom } from 'rxjs/operator/withLatestFrom';
 import { scan } from 'rxjs/operator/scan';
 import { ActionsSubject } from './actions_subject';
 import { Action, ActionReducer } from './models';
 import { INITIAL_STATE } from './tokens';
 import { ReducerObservable } from './reducer_manager';
 import { ScannedActionsSubject } from './scanned_actions_subject';
+import { INIT } from './';


Add INIT to existing imports from ./actions_subject

I'm sorry I don't know what you mean.

On line 9, add INIT to the existing object

import { ActionsSubject, INIT } from './actions_subject';

Haha, got it. It took me an embarrassingly long time to figure out what you were referring to.

brandonroberts · 2017-07-19T01:22:30Z

modules/store/spec/utils.spec.ts

+  describe(`combineReducers()`, () => {
+    const s1 = { x: '' };
+    const s2 = { y: '' };
+    const r1 = (s = s1, a: any): typeof s1 =>


Just a preference, but make the variable names more descriptive than s and a so they are more readable at glance

bfricka · 2017-07-19T01:44:31Z

Not sure why Circle failed. Everything runs fine here.

coveralls · 2017-07-19T01:55:07Z

Coverage increased (+1.1%) to 89.665% when pulling 50d55b2 on bfricka:test/add-specs into 920a5f4 on ngrx:master.

- (BREAKING) OpaqueToken -> InjectionToken - Stronger types for core actions (INIT, UPDATE) - Add more tests

coveralls · 2017-07-19T02:10:45Z

Coverage increased (+1.1%) to 89.645% when pulling 57130fd on bfricka:test/add-specs into 920a5f4 on ngrx:master.

brandonroberts · 2017-07-19T02:24:40Z

LGTM

bfricka force-pushed the test/add-specs branch from cee3302 to bb489e8 Compare July 16, 2017 07:10

MikeRyanDev assigned brandonroberts Jul 16, 2017

test(platform): Add spec watcher and unit tests

24565e2

- Add watch:tests for simpler dev / test - Store utils spec

bfricka force-pushed the test/add-specs branch from bb489e8 to 7f59f72 Compare July 18, 2017 22:13

brandonroberts requested changes Jul 19, 2017

View reviewed changes

bfricka force-pushed the test/add-specs branch from 7f59f72 to dbb3cff Compare July 19, 2017 01:36

bfricka force-pushed the test/add-specs branch from dbb3cff to 50d55b2 Compare July 19, 2017 01:52

feat(store): Add initialState fn to features

57130fd

- (BREAKING) OpaqueToken -> InjectionToken - Stronger types for core actions (INIT, UPDATE) - Add more tests

bfricka force-pushed the test/add-specs branch from 50d55b2 to 57130fd Compare July 19, 2017 02:08

brandonroberts approved these changes Jul 19, 2017

View reviewed changes

brandonroberts merged commit 5e5d7dd into ngrx:master Jul 19, 2017

bfricka deleted the test/add-specs branch July 21, 2017 02:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test/add specs #85

Test/add specs #85

bfricka commented Jul 16, 2017

coveralls commented Jul 16, 2017

bfricka commented Jul 18, 2017

coveralls commented Jul 18, 2017

brandonroberts Jul 19, 2017

brandonroberts Jul 19, 2017

brandonroberts Jul 19, 2017

brandonroberts Jul 19, 2017

bfricka Jul 19, 2017

brandonroberts Jul 19, 2017

brandonroberts Jul 19, 2017

bfricka Jul 19, 2017

brandonroberts Jul 19, 2017

bfricka Jul 19, 2017

brandonroberts Jul 19, 2017

bfricka commented Jul 19, 2017

coveralls commented Jul 19, 2017

coveralls commented Jul 19, 2017

brandonroberts commented Jul 19, 2017

Test/add specs #85

Test/add specs #85

Conversation

bfricka commented Jul 16, 2017

coveralls commented Jul 16, 2017

bfricka commented Jul 18, 2017

coveralls commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bfricka commented Jul 19, 2017

coveralls commented Jul 19, 2017

coveralls commented Jul 19, 2017

brandonroberts commented Jul 19, 2017