Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

rewrite tests to use intern #555

Closed
wants to merge 1 commit into from

Conversation

patrickkettner
Copy link

fixes #489...sorta

I changed the testing to use intern's QUnit's interface. It will still need to have a browserstack API (tested locally with mine, and it works).

For this to be actually useful, something akin to https://github.com/twbs/savage would need to be added for PRs. In addition, for the actual ask of #489 to be fulfilled, some kind of box that auto downloads the latest versions of browsers and runs this against them. Travis wouldn't work well for this, since its essentially linux only (there is os x support, but its invite only and really only meant for iphone apps). AppVeyor could work, since it runs on windows, but it would be an fairly non standard env. So a mac mini or custom windows hosting somewhere where chrome canary, firefox nightly, webkit nightly, and edge could be installed would probably be best (imo, at least).

@aFarkas
Copy link
Collaborator

aFarkas commented Jul 24, 2015

@patrickkettner
Thx for your work. I had no time yet to go through, but picturefill has the need to be a named module (patrickkettner@8f5782e#diff-e3cc12fee32d40e95863f7b6bee3e282L1419). We can't change this. Is there another way to integrate intern tests with picturefill as a named amd module?

@patrickkettner
Copy link
Author

Hiya!

There are other ways to get it to work, using an explicit name is sort of a
requirejs antipattern (
https://github.com/jrburke/requirejs/wiki/Updating-existing-libraries#anon),
so I removed it.

I believe adding an empty dependency array to to and signature will make it
work with a name, though

On Fri, Jul 24, 2015, 4:05 AM Alexander Farkas notifications@github.com
wrote:

@patrickkettner https://github.com/patrickkettner
Thx for your work. I had no time yet to go through, but picturefill has
the need to be a named module (patrickkettner/picturefill@8f5782e
#diff-e3cc12fee32d40e95863f7b6bee3e282L1419
patrickkettner@8f5782e#diff-e3cc12fee32d40e95863f7b6bee3e282L1419).
We can't change this. Is there another way to integrate intern tests with
picturefill as a named amd module?


Reply to this email directly or view it on GitHub
#555 (comment)
.

@aFarkas
Copy link
Collaborator

aFarkas commented Jul 24, 2015

@patrickkettner

using an explicit name is sort of a requirejs antipattern

We are aware of this. But we recommend to add picturefill async in the head, often separated from the rest of the JS (similar to Modernizr). This can produce errors, if requireJS is in the page and the module is unnamed.

I already checked today more of your work and found some errors and there are still some open questions. But I need to clarify some parts and need to play around first. I would suggest, that I'm trying to fix those in your forked branch so we can then pull everything in at once, if and as soon as the picturefill team decides to do so.

Will start with this at Sunday/Monday.

@patrickkettner
Copy link
Author

I would suggest, that I'm trying to fix those in your forked branch... Will start with this at Sunday/Monday

I can work on any issue today and saturday, so any issue you can mention before then would be appreciated.

@aFarkas
Copy link
Collaborator

aFarkas commented Jul 24, 2015

In general: I have added a new index.html into the tests director:

<!DOCTYPE html>
<title>Wait a minute, I'll redirect you</title>
<meta http-equiv="refresh" content="0; url=../node_modules/intern/client.html?config=tests/intern&reporters=Html&reporters=Console">

Open this in current Safari

Here are things I saw (not everything an issue)

  1. making it worked with named module
  2. picture has to be created first: https://github.com/patrickkettner/picturefill/blob/new_tests/tests/tests.js#L117-L122
  3. I think we should keep qunit for tests-functional.html (patrickkettner@8f5782e#diff-595bf3fa2348192244b0319be33066b8L62 patrickkettner@8f5782e#diff-b9cfc7f2cdf78a7f4b91a753d10865a2L34).
  4. I don't understand why removing this: patrickkettner@8f5782e#diff-2a8a5fef3397df87ab538f028a5c6b50L68 works.

@aFarkas
Copy link
Collaborator

aFarkas commented Jul 24, 2015

Sorry for being soo short. I have no time. Thank you!!! Really great.

@patrickkettner
Copy link
Author

I think we should keep qunit for tests-functional.html

Any reason you don't want to use intern for functional tests as well?

@patrickkettner
Copy link
Author

@aFarkas I believe I updated all of your concerns

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants