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

Need Help for testing #47

Closed
wcastand opened this issue Dec 8, 2015 · 29 comments
Closed

Need Help for testing #47

wcastand opened this issue Dec 8, 2015 · 29 comments
Labels

Comments

@wcastand
Copy link
Contributor

wcastand commented Dec 8, 2015

Hi, i wanted to contribute so i decide to test the Events.
But i can't figure it out what i'm doing wrong here :

describe("$.delegate", function() {
    it("exists", function() {
        expect($.delegate).to.exist;
    });
    it("add event on children of subject", function() {
        var test = false;
        var subject = document.createElement("ul");
        subject.id = "tete";
        var inside = document.createElement("li");
        subject.appendChild(inside);

        document.body.appendChild(subject);
        $.delegate(subject, "click", {
            li: function(){
                console.log("te");
                test = true;
            }
        });
        $('#tete li').click();
        expect(test).to.be.equal(true);
    });
});

My file called "DelegateSpec.js" in the folder "tests/Events/" and i get this error by karma:

TypeError: 'undefined' is not a function (evaluating '$('#tete li').click()')
            at F:/Web/bliss/tests/Events/DelegateSpec.js:19

I'm quite new at testing (and already hate that :-P), so be nice with me please :-)

@LeaVerou
Copy link
Owner

LeaVerou commented Dec 8, 2015

Hey there, thanks for contributing!
There is no click method on li elements. Only some types of elements have this natively (e.g. a elements).
If you were trying to fire a click event on the element, change it to:

$('#tete li')._.fire("click");

(in a rush so haven’t checked the rest of the code)

@wcastand
Copy link
Contributor Author

wcastand commented Dec 8, 2015

Thank for the answer.
I guess chrome add itself the click function on all the elements because it worked on my test to understand delegate in chrome :)

When i use the fire function, i have no error but it didn't fire the click function from the $.delegate
I used the same code as my first post, except the line :

$('#tete li').click();
//replace by
$('#tete li')._.fire("click");

I don't know if i have to open an other issue. But when i try to create an element with the property 'events', i get an error:

$.create("a", {
    href: "#here",
    title: "Permalink",
    className: "permalink",
    events: {
        click: function(evt) {
            console.log("test")
        }
    }
});
ReferenceError: Can't find variable: EventTarget
            at F:/Web/bliss/bliss.js:408
            at F:/Web/bliss/bliss.js:328
            at F:/Web/bliss/bliss.js:566
            at F:/Web/bliss/bliss.js:588
            at F:/Web/bliss/bliss.js:88
            at F:/Web/bliss/tests/Events/DelegateSpec.js:15

@LeaVerou
Copy link
Owner

LeaVerou commented Dec 8, 2015

I think you're using an older version. Please pull?

@wcastand
Copy link
Contributor Author

wcastand commented Dec 8, 2015

Ok, i update my fork and it works. My bad here

Anyway for the test of delegate, it still doesn't work.

it("add event on children of subject", function() {
        var test = false;
        var subject = document.createElement("div");
        subject.id = "tete";
        var aa = $.create("a", {
            href: "#here",
            title: "Permalink",
            className: "permalink",
            events: {
                click: function(evt) {
                    console.log("testa")
                }
            }
        });
        subject.appendChild(aa)
        document.body.appendChild(subject);
        var tt = subject._.delegate("click",
        {
            a: function(){
                console.log("tea");
                test = true;
            }
        });
        $('#tete a')._.fire('click');
        expect(test).to.be.equal(true);
    });
LOG: 'testa'
PhantomJS 1.9.8 (Windows 8 0.0.0) $.delegate add event on children of subject FAILED

So when i fire 'click', the callback from the $.create works but the one from delegate doesn't.
I try to remove the event from the $.create and the result is the same.
Maybe i didn't understand how to delegate function work :/

guoguo12 added a commit to guoguo12/bliss that referenced this issue Dec 8, 2015
Based on a test by @wcastand in LeaVerou#47. (Thanks!)

This seems to fail on PhantomJS due because Element.matches (used in
$.delegate) is unavaiable. Changing $.delegate to use
Element.webkitMatchesSelector makes the test pass.

This test passes as is on Chrome and Firefox Developer.
guoguo12 added a commit to guoguo12/bliss that referenced this issue Dec 8, 2015
Based on a test by @wcastand in LeaVerou#47. (Thanks!)

This seems to fail on PhantomJS due because Element.matches (used in
$.delegate) is unavailable. Changing $.delegate to use
Element.webkitMatchesSelector makes the test pass.

This test passes as is on Chrome and Firefox Developer.
@guoguo12
Copy link
Collaborator

guoguo12 commented Dec 8, 2015

So I rewrote the test to work independently of other Bliss methods. (I hope you don't mind, @wcastand.)

I think the issue here is that $.delegate legitimately doesn't work on PhantomJS. When I run my test, I'll get an error like

PhantomJS 1.9.8 (Windows 8 0.0.0) $.delegate adds event to children of the subject FAILED
        TypeError: 'undefined' is not a function (evaluating 'evt.target.matches(selector)') (C:/Users/guogu_000/Git/bliss/bliss.js:480)

indicating that Element.matches isn't valid in PhantomJS. Replacing Element.matches with Element.webkitMatchesSelector makes the test pass on PhantomJS.

[Note: I force-pushed over commit 39c214e, so that's why it shows up twice. Didn't know the mention wouldn't go away. Sorry!]

@wcastand
Copy link
Contributor Author

wcastand commented Dec 8, 2015

Hi,
I don't mind at all. Thanks for the help !
I will continue to test the others methods of Events.

@zdfs zdfs added the tests label Dec 8, 2015
@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

This begs the question, should we have tests that only execute in a real browser and tests that execute in a headless env? The strategy being that we try to make something work in Phantom, and if we can't, save it for when the developer is using a real browser.

Just throwing out ideas.

@guoguo12
Copy link
Collaborator

guoguo12 commented Dec 8, 2015

Honestly, I wouldn't mind. PhantomJS is being really buggy for me on watch mode. I'll karma start and everything will pass, and then I'll save one of the test files without changing anything and PhantomJS will fail. Am I missing something about how PhantomJS operates during watch mode?

@wcastand
Copy link
Contributor Author

wcastand commented Dec 8, 2015

I actually had the same issue when i was trying to figure it out delegate. Sometime my test just didn't pass on my DelegateSpec.js and at one point i get an error on a bliss file. i just save the file again and the error disappear.
I didn't understand why.

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Actually, maybe we can use Firefox. Travis-ci seems to have support for that.

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

I'm going to change our config to Firefox, and see if Travis builds. Probably better if we use a real browser if Travis supports one.

zdfs pushed a commit that referenced this issue Dec 8, 2015
zdfs pushed a commit that referenced this issue Dec 8, 2015
This seems to be working completely on my local now, hopefully
Travis-CI follows.
@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Ok. I think I'm almost there.

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Ok. It seems to be running the tests with Firefox now, but can anyone tell me if it's really running the tests or not. It seems like it is, but the counts are off: https://travis-ci.org/LeaVerou/bliss/jobs/95623030

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

According to the screenshot here (http://www.sitepoint.com/testing-javascript-jasmine-travis-karma/), it seems like that's the normal behavior.

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

And I see 27 SUCCESS lines. So I think we can have a normal test now instead of some special PhantomJS version. @wcastand and @guoguo12, is there any cleanup we need to do since we have a Firefox test environment on Travis now?

@guoguo12
Copy link
Collaborator

guoguo12 commented Dec 8, 2015

Okay, great. I don't think there's any cleanup that needs to be done.

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Close this one out then?

@guoguo12
Copy link
Collaborator

guoguo12 commented Dec 8, 2015

Sure.

@LeaVerou
Copy link
Owner

LeaVerou commented Dec 8, 2015

Thank you all!
Sorry if this is a silly question, but why Firefox and not Chrome? AFAIK there is a Chrome launcher too…

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Firefox is already on Travis. From what I've read, it seemed like I would need steps to build and install Chrome.

I can try Chrome and see what happens. We can always go back to Firefox.

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

I'm curious now. Going to try it.

@LeaVerou
Copy link
Owner

LeaVerou commented Dec 8, 2015

Given that Chrome has a much higher market share, I’d prioritize that. But if it's too much of a hassle, Firefox is fine!

zdfs pushed a commit that referenced this issue Dec 8, 2015
@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Ok. Looks like we can't start Chrome. I can get this to work. I've done it on Wercker. But it's going to take me a few tries. I don't mind.

zdfs pushed a commit that referenced this issue Dec 8, 2015
@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Holy shit.

screen shot 2015-12-08 at 11 20 52 am

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

Chrome is set. Closing this out.

@zdfs zdfs closed this as completed Dec 8, 2015
@guoguo12
Copy link
Collaborator

guoguo12 commented Dec 8, 2015

Good work!

@dperrymorrow
Copy link
Collaborator

@zdfs if we are going to run the suite in Chrome, should i back out that change i made to get $.all to run in headless mode?

the expr.constructor.name vs instanceof business

@zdfs
Copy link
Collaborator

zdfs commented Dec 8, 2015

You can do that. I can merge something like that as opposed to changes that affect core code.

@zdfs zdfs mentioned this issue Dec 8, 2015
dperrymorrow pushed a commit to dperrymorrow/bliss that referenced this issue Dec 8, 2015
Based on a test by @wcastand in LeaVerou#47. (Thanks!)

This seems to fail on PhantomJS due because Element.matches (used in
$.delegate) is unavailable. Changing $.delegate to use
Element.webkitMatchesSelector makes the test pass.

This test passes as is on Chrome and Firefox Developer.
dperrymorrow pushed a commit to dperrymorrow/bliss that referenced this issue Dec 8, 2015
dperrymorrow pushed a commit to dperrymorrow/bliss that referenced this issue Dec 8, 2015
@LeaVerou
Copy link
Owner

LeaVerou commented Dec 9, 2015

Thank you so much @zdfs!

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

No branches or pull requests

5 participants