Fix failing tests across browsers

[cvle]

Related https://github.com/cssinjs/jss/issues/89

[cvle]

I moved the test file into a separate folder, and I'm trying to adapt the scripts in package.json, but I don't really get the purpose of npm run build:tests ?

[cvle]

Everything else is ready.

[cvle]

@kof I did some changes in the last hour, in case you were already checking out this PR.

[cvle]

I'm finally done. This one was really tricky.

Currently mocha report suffers from this issue: https://github.com/litixsoft/karma-mocha-reporter/issues/84.

[kof]

I think I saw that bug as well, but didn't dig to understand whats going on there.

[cvle]

I just fixed some remaining linter errors.

[kof]

Well we can refactor file names any time, its easy. For now I started to use that fileName.test.js convention in all plugins where we don't separate integration and unit tests. Also if they are small integration and unit tests can be all in the same file, normally people separate them in large projects when speed of running tests becomes a problem.

On Nov 21, 2016, at 3:41 AM, Chi Vinh Le notifications@github.com wrote:

@cvle commented on this pull request.

In tests.webpack.js https://github.com/cssinjs/css-vendor/pull/9:

@@ -1,2 +1,2 @@ -const context = require.context('./src', true, /.test.js$/) +const context = require.context('./test', true, /.test.js$/) Why do you want to keep the file in src/index.test.js? For me this indicates that it is an unit test only for the index file, but it is an integration test for the whole project. For me that's a bit confusing. What if you were to add unit tests in the future?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cssinjs/css-vendor/pull/9, or mute the thread https://github.com/notifications/unsubscribe-auth/AADOWP3X8Kf8saudGFXK-LNzLG69CVVpks5rAQTkgaJpZM4K3foh.

[cvle]

I see, I was just curious for the reason.

[cvle]

Almost done. Waiting for some free slots on your browserstack account to fix remaining issues if any.

[cvle]

Everything passed! 👍
This is good to go.

Check out the new package at https://github.com/wikiwi/caniuse-support.

[kof]

Awesome

[kof]

Hmmm ok then.

On Nov 23, 2016, at 12:14 PM, Chi Vinh Le notifications@github.com wrote:

@cvle commented on this pull request.

In karma.conf.js https://github.com/cssinjs/css-vendor/pull/9:

 })

 config.browserStack = {
   username: browserStackUserName,
   accessKey: browserStackAccessKey,

• captureTimeout: 10000
• captureTimeout: 3e5 See this issue: karma-runner/karma-browserstack-launcher#61 https://github.com/karma-runner/karma-browserstack-launcher/issues/61. I had issues with testing on the mobile android device which got restarted on every run and then get stuck forever. I had to kill the worker on each run manually in order to free the slot again. I took the values from the issue above, which are supposed to be timeout values given by the browserstack support.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cssinjs/css-vendor/pull/9, or mute the thread https://github.com/notifications/unsubscribe-auth/AADOWMUPVqZlnGd3uFAtutdNkGzivP-nks5rBCAGgaJpZM4K3foh.

[kof]

Looks good. We can merge it. One more idea would be to generate tests for every known property and do those checks. This way we know if anything else goes wrong.

[cvle]

What we can do is to map property names to the appropriate caniuse feature name, and use this to run our prefix test. Here is a list we could rely on for which properties to test: http://shouldiprefix.com/

Let's leave this up to a following PR, and merge this first. Suggestions have been applied.

[kof]

merged

[cvle]

Just looking at the travis build, I noticed that currently npm install triggers prepublish which runs npm run all, so the travis test runs browser stack twice and everyone tinkering with this package also runs karma test during install.

[kof]

travis test runs browser stack twice

u sure? It should only do install which runs the tests

so the travis test runs browser stack twice and everyone testing this package also runs karma test during install.

yes, but only if you are installing it in a cloned repository, not if it is a dependency, so it is only for css-vendor developers the case

[kof]

I think npm recently fixed that by adding a separate hook for what we need, the intend was always to test before publishing, not all the other cases. Also there is a separate package which solves this problem. Need to do that for all repos though.

[cvle]

Travis just ran test twice during npm install and npm test checkout the logs.

[kof]

hmm, where does install and test come from, it should be just install

[cvle]

Alternatively you can think about publishing using travis. My approach currently looks like this:

• Run ~npm release patch~ npm version patch
• Run git push --follow-tags
• Let travis deploy the package.

[cvle]

hmm, where does install and test come from, it should be just install

That's the default travis behavior.

[kof]

Shit))) we need to fix that, running tests twice on jss repo as well then. And yes, seems to be a good idea to publish the package over travis.

[kof]

Oh actually we may be able to say what to run in travis.yaml.

[cvle]

Yes, if you define the script part, travis will not apply its default behavior.

[kof]

yeah, so we need either this or publish over travis. I tend to publish over travis and remove the prepublish thing then.

[cvle]

I recommend publishing over travis too 👍

[cvle]

My mistake before: I wrote npm release patch but it is npm version patch. Checkout my project https://github.com/wikiwi/caniuse-support to get some ideas.

E.g. in this project I just run npm run release patch && git push --follow-tags for a patch release.