jonathanKingston / ember-cli-sri

Generation of SRI hashes for ember
MIT License
48 stars 15 forks source link

testem.js causes Chrome Canary to not auto-reload tests #2

Closed blimmer closed 8 years ago

blimmer commented 8 years ago

This might not be the right place to file this, but it can at least be a jumping off point if I need to report to ember-cli/testem/etc.

With SRI integrity added to testem.js, Chrome Canary refuses to load it:

Failed to find a valid digest in the 'integrity' attribute for resource 'http://localhost:7357/testem.js' with computed SHA-256 integrity 'lh8NuSGzYSb6XpdfBYsEglnfxBdd6LpuYybvxZC97fM='. The resource has been blocked.
jonathanKingston commented 8 years ago

@blimmer bahah yeah that is a good catch (I stopped using the browser on my dev machine a while back due to it being way too slow).

Do you think turning it off would be a good idea? Obviously then it wouldn't be then testing the SRI; so it would probably be a worthwhile quick fix.

Let me give it some thought as more projects use live reloading and it would be better to fix this before SRI becomes more prevalent. (The internals of broccoli-sri-hash are being moved into a node friendly script that every FE dev can use in their build processes)

blimmer commented 8 years ago

I think turning off SRI for those scripts for dev-only purposes is a good idea. That said, not really sure how to implement that :-X

jonathanKingston commented 8 years ago

Yeah I will sort that and submit a pull (probably here and in Ember).

However it would be nice to test with SRI if people want to... at the end of the day if code breaks (I can't see how that would happen; but for paranoid me I would like to)

jonathanKingston commented 8 years ago

The actual issue is the testem.js is dynamically created.

Changing the file tests/index.html to the following fixes the issue: <script src="testem.js" integrity="" ></script>

@stefanpenner I can make this a pull request, however again I'm not really sure the best way to test this. I'm thinking checking that if a file loaded by http/s is different to the file path lookup in a generic sense for tests and application should fail a test if it is loaded without a integrity="". (My addon looks at the file path). Tips on how this should be written would be great.

stefanpenner commented 8 years ago

I would just test for the empty integrity attribute. As CI doesn't use a browser that supports SRI yet.

jonathanKingston commented 8 years ago

Yeah that was my thinking too, even when it does it won't be reliable for a while either.

I was more asking you @stefanpenner as I wasn't sure if it should be a blueprint check or what? Surely testing the test/index.html can only be done at the generator level, or am I mistaken?

Keeo commented 8 years ago

+1 I got same error on production environment.

Failed to find a valid digest in the 'integrity' attribute for resource 'http://example.com/assets/vendor-ab160f374b260ada660f3fa9caedde96.js' with computed SHA-256 integrity '76vh6KM_KG_S2BQ11lLwJLI8rY9vQQu4BWz4aqBrdPw='. The resource has been blocked.
Uncaught ReferenceError: define is not defined.

And when I look into the file, it starts with "use strict";define("project/acceptance-tests/main" Shouldn't tests be completely disabled for production build?

jonathanKingston commented 8 years ago

@Keeo I think your issue is separate this is specifically just a file within testem. I think there are two issues you may be facing. One which a blank acceptance file gets merged into the main (looking into this). And another which could be an encoding issue perhaps. Are you serving your code with UTF8?

john-kurkowski commented 8 years ago

This now happens in Chrome 45 stable.

jonathanKingston commented 8 years ago

@john-kurkowski please use the <script src="testem.js" integrity="" ></script> in the tests/index.html file. All other issues should be reported as a separate issue.

(The only thing holding this going into the main generator is writing a decent generator test which I'm nor sure about the best approach for as of yet)

jonathanKingston commented 8 years ago

Resolved by this: https://github.com/ember-cli/ember-cli/pull/4796