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

build: Add Store unit tests to Bazel #836

Merged
merged 1 commit into from
Feb 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
# Marker file indicating this folder is a Bazel package.
# Needed so that tsconfig.json can be referenced from BUILD rules.
package(default_visibility = ["//visibility:public"])

# Needed so that tsconfig.json can be referenced from BUILD rules.
exports_files(["tsconfig.json"])

# Temporary target to allow `bazel test ...`
# TODO(alexeagle): remove as soon as there is any other test in the repo
genrule(
name = "true",
outs = ["true.sh"],
cmd = "echo true > $@",
)

sh_test(
name = "tautology_test",
srcs = [":true.sh"],
filegroup(
Copy link
Member Author

@MikeRyanDev MikeRyanDev Feb 17, 2018

Choose a reason for hiding this comment

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

It isn't clear to me if I should be using the dependencies installed from running Yarn at the root or if I should do something similar to the ngrx_compile_time_deps for tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

You have both options.
In the case of tests, your users probably don't expect to build/run these from source. At the same time, your users would download and install the ngrx_compiletime_deps so it's not nice to add test-only dependencies there.
I think I'd probably re-use the node_modules directory installed by yarn (note you can use bazel run @yarn//:yarn to do that with Bazel's version of node and yarn).

Copy link
Member Author

@MikeRyanDev MikeRyanDev Feb 18, 2018

Choose a reason for hiding this comment

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

Good deal. This PR follows your suggestion and re-uses the top level node_modules directory for test dependencies.

name = "ngrx_test_dependencies",
# NB: rxjs is not in this list, because we build it from sources using the
# label @rxjs//:rxjs
srcs = glob(["/".join([
"node_modules",
pkg,
"**",
ext,
]) for pkg in [
"@angular",
"jasmine",
"jasmine-marbles",
"typescript",
"@types",
] for ext in [
"*.js",
"*.json",
"*.d.ts",
]]),
)
24 changes: 24 additions & 0 deletions modules/store/spec/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
load("//tools:defaults.bzl", "ts_test_library", "jasmine_node_test")

ts_test_library(
name = "test_lib",
srcs = glob(
[
"**/*.ts",
],
exclude = ["ngc/**/*.ts"],
),
deps = [
"//modules/store",
"@rxjs",
],
)

jasmine_node_test(
name = "test",
deps = [
":test_lib",
"//modules/store",
"@rxjs",
],
)
14 changes: 8 additions & 6 deletions modules/store/spec/edge.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { TestBed } from '@angular/core/testing';
import { Observable } from 'rxjs/Observable';
import { todos, todoCount } from './fixtures/edge_todos';
import { createInjector } from './helpers/injector';
import { Store, StoreModule, select } from '../';
import { Store, StoreModule, select } from '@ngrx/store';

interface TestAppSchema {
counter1: number;
Expand All @@ -21,11 +21,13 @@ describe('ngRx Store', () => {
let store: Store<TodoAppSchema>;

beforeEach(() => {
const injector = createInjector(
StoreModule.forRoot<TodoAppSchema>({ todos, todoCount } as any)
);
TestBed.configureTestingModule({
imports: [
StoreModule.forRoot<TodoAppSchema>({ todos, todoCount } as any),
],
});

store = injector.get(Store);
store = TestBed.get(Store);
});

it('should provide an Observable Store', () => {
Expand Down
29 changes: 0 additions & 29 deletions modules/store/spec/helpers/injector.ts

This file was deleted.

2 changes: 1 addition & 1 deletion modules/store/spec/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
ActionReducer,
ActionReducerMap,
select,
} from '../';
} from '@ngrx/store';
import { ReducerManager, INITIAL_STATE, State } from '../src/private_export';
import {
counterReducer,
Expand Down
17 changes: 8 additions & 9 deletions modules/store/spec/modules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import {
ActionReducer,
ActionReducerMap,
combineReducers,
} from '../';
import createSpy = jasmine.createSpy;
} from '@ngrx/store';

describe(`Store Modules`, () => {
type RootState = { fruit: string };
Expand Down Expand Up @@ -48,14 +47,14 @@ describe(`Store Modules`, () => {
const rootInitial = { fruit: 'orange' };

beforeEach(() => {
featureAReducerFactory = createSpy('featureAReducerFactory').and.callFake(
(rm: any, initialState?: any) => {
featureAReducerFactory = jasmine
.createSpy('featureAReducerFactory')
.and.callFake((rm: any, initialState?: any) => {
return (state: any, action: any) => 4;
}
);
rootReducerFactory = createSpy('rootReducerFactory').and.callFake(
combineReducers
);
});
rootReducerFactory = jasmine
.createSpy('rootReducerFactory')
.and.callFake(combineReducers);

@NgModule({
imports: [
Expand Down
2 changes: 1 addition & 1 deletion modules/store/spec/selector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
createFeatureSelector,
defaultMemoize,
createSelectorFactory,
} from '../';
} from '@ngrx/store';

describe('Selectors', () => {
let countOne: number;
Expand Down
21 changes: 11 additions & 10 deletions modules/store/spec/state.spec.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
import { Observable } from 'rxjs/Observable';
import { Subject } from 'rxjs/Subject';
import { ReflectiveInjector } from '@angular/core';
import { createInjector } from './helpers/injector';
import { StoreModule, Store, INIT } from '../';
import { TestBed } from '@angular/core/testing';
import { StoreModule, Store, INIT } from '@ngrx/store';

describe('ngRx State', () => {
const initialState = 123;
const reducer = jasmine.createSpy('reducer').and.returnValue(initialState);
let injector: ReflectiveInjector;

beforeEach(() => {
injector = createInjector(
StoreModule.forRoot(
{ key: reducer },
{ initialState: { key: initialState } }
)
);
TestBed.configureTestingModule({
imports: [
StoreModule.forRoot(
{ key: reducer },
{ initialState: { key: initialState } }
),
],
});
});

it('should call the reducer to scan over the dispatcher', function() {
injector.get(Store);
TestBed.get(Store);

expect(reducer).toHaveBeenCalledWith(initialState, {
type: INIT,
Expand Down
14 changes: 8 additions & 6 deletions modules/store/spec/store.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'rxjs/add/operator/take';
import { ReflectiveInjector } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { hot } from 'jasmine-marbles';
import { createInjector } from './helpers/injector';
import {
ActionsSubject,
ReducerManager,
Expand All @@ -26,7 +26,6 @@ interface TestAppSchema {
}

describe('ngRx Store', () => {
let injector: ReflectiveInjector;
let store: Store<TestAppSchema>;
let dispatcher: ActionsSubject;

Expand All @@ -37,9 +36,12 @@ describe('ngRx Store', () => {
counter3: counterReducer,
};

injector = createInjector(StoreModule.forRoot(reducers, { initialState }));
store = injector.get(Store);
dispatcher = injector.get(ActionsSubject);
TestBed.configureTestingModule({
imports: [StoreModule.forRoot(reducers, { initialState })],
});

store = TestBed.get(Store);
dispatcher = TestBed.get(ActionsSubject);
}

describe('initial state', () => {
Expand Down Expand Up @@ -199,7 +201,7 @@ describe('ngRx Store', () => {

beforeEach(() => {
setup();
const reducerManager = injector.get(ReducerManager);
const reducerManager = TestBed.get(ReducerManager);
addReducerSpy = spyOn(reducerManager, 'addReducer').and.callThrough();
removeReducerSpy = spyOn(
reducerManager,
Expand Down
18 changes: 18 additions & 0 deletions tools/defaults.bzl
Original file line number Diff line number Diff line change
@@ -1,9 +1,27 @@
"""Re-export of some bazel rules with repository-wide defaults."""
load("@build_bazel_rules_typescript//:defs.bzl", _ts_library = "ts_library")
load("@build_bazel_rules_nodejs//:defs.bzl", _jasmine_node_test = "jasmine_node_test")

def ts_library(tsconfig = None, node_modules = None, **kwargs):
if not tsconfig:
tsconfig = "//:tsconfig.json"
if not node_modules:
node_modules = "@ngrx_compiletime_deps//:node_modules"
_ts_library(tsconfig = tsconfig, node_modules = node_modules, **kwargs)

def ts_test_library(node_modules = None, **kwargs):
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this an OK pattern of defining my own rules that just set defaults for existing rules similar to what you've done with ts_library? Or should I be more explicit in BUILD files?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a macro (you can tell because you didn't call the rule factory function) and macros can be the gateway to a bad time. In Google, there are a lot of macros that produce surprising combinations of targets, and make the BUILD files look very different from the result of bazel query --output=build.
This usage is okay though, matching the criteria:

  • it should only wrap one rule, not multiple
  • it should just set defaults

This looks clever and reasonable to me :)

if not node_modules:
node_modules = "//:ngrx_test_dependencies"
ts_library(node_modules = node_modules, testonly = 1, **kwargs)

def jasmine_node_test(node_modules = None, bootstrap = None, deps = [], **kwargs):
if not node_modules:
node_modules = "//:ngrx_test_dependencies"
if not bootstrap:
bootstrap = ["ngrx/tools/testing/bootstrap_node_tests.js"]
Copy link
Member Author

@MikeRyanDev MikeRyanDev Feb 17, 2018

Choose a reason for hiding this comment

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

It surprised me that I couldn't pass a label here and instead had to point it to a build artifact.

Copy link
Contributor

Choose a reason for hiding this comment

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

It ought to allow "$(location //my:label)" but doesn't yet :(
bazel-contrib/rules_nodejs#32

_jasmine_node_test(
bootstrap = bootstrap,
node_modules = node_modules,
deps = ["//tools/testing:node"] + deps,
**kwargs
)
8 changes: 8 additions & 0 deletions tools/testing/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package(default_visibility = ["//visibility:public"])

load("//tools:defaults.bzl", "ts_test_library")

ts_test_library(
name = "node",
srcs = ["bootstrap_node_tests.ts"],
)
33 changes: 33 additions & 0 deletions tools/testing/bootstrap_node_tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import 'core-js/es7/reflect';
import 'zone.js/dist/zone-node.js';
import 'zone.js/dist/long-stack-trace-zone.js';
import 'zone.js/dist/proxy.js';
import 'zone.js/dist/sync-test.js';
import 'zone.js/dist/async-test.js';
import 'zone.js/dist/fake-async-test.js';

const jasmineCore: any = require('jasmine-core');
const patchedJasmine = jasmineCore.boot(jasmineCore);
(global as any)['jasmine'] = patchedJasmine;

jasmineCore.boot = function() {
return patchedJasmine;
};

import { TestBed } from '@angular/core/testing';
import {
ServerTestingModule,
platformServerTesting,
} from '@angular/platform-server/testing';

require('zone.js/dist/jasmine-patch.js');

const originalConfigureTestingModule = TestBed.configureTestingModule;

TestBed.configureTestingModule = function() {
TestBed.resetTestingModule();

return originalConfigureTestingModule.apply(null, arguments);
};

TestBed.initTestEnvironment(ServerTestingModule, platformServerTesting());