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

Bazel testing #6

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Bazel testing #6

wants to merge 8 commits into from

Conversation

juristr
Copy link
Owner

@juristr juristr commented Feb 21, 2018

This is a WIP of adding a unit test to the setup, but doesn't yet work.

Getting some errors about loading RxJS which remember me the bad old RequireJS times 😅

21 02 2018 22:43:32.991:INFO [launcher]: Starting browser ChromeHeadless
21 02 2018 22:43:33.294:INFO [HeadlessChrome 65.0.3325 (Mac OS X 10.11.6)]: Connected on socket qfviKll2Qhp0pEWhAAAA with id 99851694
21 02 2018 22:43:33.551:WARN [web-server]: 404: /base/rxjs/Observable.js
21 02 2018 22:43:33.552:WARN [web-server]: 404: /base/rxjs/observable/merge.js
21 02 2018 22:43:33.553:WARN [web-server]: 404: /base/rxjs/operator/share.js
21 02 2018 22:43:33.554:WARN [web-server]: 404: /base/rxjs/Subject.js
21 02 2018 22:43:33.556:WARN [web-server]: 404: /base/rxjs/Subscription.js
HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR
  Script error.
  at :0:0


HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR: 'There is no timestamp for /base/rxjs/Observable.js!'

HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR: 'There is no timestamp for /base/rxjs/observable/merge.js!'

HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR: 'There is no timestamp for /base/rxjs/operator/share.js!'

HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR: 'There is no timestamp for /base/rxjs/Subject.js!'

HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR: 'There is no timestamp for /base/rxjs/Subscription.js!'

alexeagle and others added 4 commits February 21, 2018 23:05
* build: ignore bazel branch on Travis (#4)

CircleCI will be used instead

* Introduce bazel

Can now `bazel build //...` to compile the code with ngc.

* code review feedback
@juristr
Copy link
Owner Author

juristr commented Feb 22, 2018

@alexeagle ok, made some adjustments based on your angular-bazel-example and now test run more or less.

I didn't have this line

// TODO(alexeagle): this helper should be in @angular/platform-browser-dynamic/testing
try {
  TestBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting());
} catch {
  // Ignore exceptions when calling it multiple times.
}

...which you already commented should be done in the proper Angular testing module.

There's just one thing though which makes test not pass properly. It's the fact that the html and css don't get loaded properly, but are rather requested during the test-run and thus ChromeHeadless gives a 404. Inlining the HTML and CSS into the component makes the tests pass.

Is this a known issue or did I miss some step in the setup?

@alexeagle
Copy link
Collaborator

Right now the tests assume you use AOT testing, like in my example, meaning the test component loads the compiled factory. In this scenario there is no need to load css or html in the browser because it's all been run through ngc.

I've discussed with @jelbourn about adding the ability for ts_web_test to serve arbitrary data, see bazelbuild/rules_typescript#131
but I think our preferred approach is to avoid running the Angular compiler in the browser ever

@alexeagle
Copy link
Collaborator

@vikerman is there some public doc about how to convert your Angular tests to the AOT style?

@juristr
Copy link
Owner Author

juristr commented Feb 23, 2018

I understand. In fact, I've seen you're importing the factory class in the example.

but I think our preferred approach is to avoid running the Angular compiler in the browser ever

Makes sense. But is the idea to add a compilation step somehow? Otherwise it would mean that you have to rewrite/adjust your existing tests whenever you switch to Bazel.

@alexeagle
Copy link
Collaborator

For this repo, you're writing new tests so I believe the new style is preferred.
We have this problem internally at Google also - we have to serve assets to karma tests because most test code was written to use JIT.
I'll have to discuss with @vikerman whether we have any plans to write tooling to migrate test code to the new pattern.
Even if we did, I still need to make ts_web_test serve static resources, if nothing else than because Angular has some tests that load json data.

@juristr
Copy link
Owner Author

juristr commented Feb 23, 2018

I understand.

Even if we did, I still need to make ts_web_test serve static resources, if nothing else than because Angular has some tests that load json data.

Oh true that as well. It's also a common scenario.

Just updated the branch, now I'm getting a weird error that it cannot load the ngsummary files for @angular/common. Have to verify that one.

Btw, did you succeed with ng_package already?

@alexeagle
Copy link
Collaborator

If you are missing node_modules/@angular/common/*.ngsummary.json this means the ngc in the postinstall step did not do its job. Under Bazel we assume that every library has ngc compiler metadata, including those you fetched from npm.

I am still waiting for @IgorMinar to approve angular/angular#22221 so we can use ng_package... :/

@juristr
Copy link
Owner Author

juristr commented Feb 23, 2018

If you are missing node_modules/@angular/common/*.ngsummary.json this means the ngc in the postinstall step did not do its job. Under Bazel we assume that every library has ngc compiler metadata, including those you fetched from npm.

Thought that as well. But the ngc compiler seems to have done its job. I see *.ngfactory.js files as well as *.ngsummary.json files in @angular/common. It's more like they're not properly served to the web server. I'm currently trying to figure out the difference to your example repo, where it seems to work 🤔.

Also it looks for *.ngsummary.js.

HeadlessChrome 65.0.3325 (Mac OS X 10.11.6) ERROR: 'There is no timestamp for /base/ngx_tabs_libdemo/node_modules/@angular/common/common.ngsummary.js!'

Shouldn't that be ngfactory.js?

@alexeagle
Copy link
Collaborator

alexeagle commented Feb 23, 2018 via email

@juristr
Copy link
Owner Author

juristr commented Feb 23, 2018

Hmm...I removed the imports from @angular/platform-browser-dynamic/testing whichI had copied over from your bazel example repo and replaced them with the corresponding ones from the @angular/platform-browser/testing package.

605651c

However that didn't change much. Any idea what else could I be missing? The error is still the same, as if I hadn't changed anything.

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.

2 participants