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

Jump to bottom

Asynchronous step function that does not modify this value #7182

Closed
wpt-issue-mover opened this issue Aug 31, 2017 · 10 comments
Closed

Asynchronous step function that does not modify this value #7182

wpt-issue-mover opened this issue Aug 31, 2017 · 10 comments
Labels
infra priority:backlog testharness.js

Comments

@wpt-issue-mover
Copy link

Originally posted as w3c/testharness.js#236 by @JosephPecoraro on 30 Jan 2017, 22:46 UTC:

When attempting to write an async test, t.step_func is useful for wrapping a callback function, however it modifies the this value within the callback. I'd like to add a variant that doesn't modify the this value, t.step_func_unmodified_this.

Implementation (simplified)

Test.prototype.step_func_unmodified_this = function(func)
{
    var t = this;
    return function() {
        return t.step.apply(test_this, [func, this].concat(Array.from(arguments));
    };
}

Docs

For asynchronous callbacks where the this value is significant, step_func_unmodified_this can be used. For example:

var observer = new Observer(t.step_func_unmodified_this(function() {
    assert_true(this instanceof Observer, "this value must be Observer");
    t.done();
}));

Currently the above test would need to be written something like:

var finish = t.step_func(function() { t.done(); });
var observer = new Observer(function() {
    var self = this;
    t.step(function() {
        assert_true(self instanceof Observer, "this value must be Observer");
        finish();
    });
});

Which is quite obtuse, and requires using a different name for this even though this is what is being tested.
@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @gsnedders on 31 Jan 2017, 00:30 UTC:

What does step_func have as this when it does call?

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @JosephPecoraro on 31 Jan 2017, 00:42 UTC:

The this value is test object created by async_test:

let x = async_test(function(t) {
    setTimeout(t.step_func(function() {
        console.log(this === x); // true
        console.log(this === t); // true
        t.done();
    }));
}, "Title");

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @JosephPecoraro on 31 Jan 2017, 00:47 UTC:

You could change Test.prototype.step_func to not modify the this value if an explicit this_obj was not provided.

Something like: (untested)

Test.prototype.step_func = function(func, this_obj)
{
    var test_this = this;
    var has_explicit_this_obj = arguments.length > 1;

    return function()
    {
        var func_this = has_explicit_this_obj ? this_obj : this;
        return test_this.step.apply(test_this, [func, func_this].concat(
            Array.prototype.slice.call(arguments)));
    };
};

I wasn't sure if anything relied on this behavior (I only just started using testharness.js today).

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @gsnedders on 31 Jan 2017, 00:59 UTC:

I wasn't sure if anything relied on this behavior (I only just started using testharness.js today).

I have no idea! I'm wondering if we can get away with that. :)

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @JosephPecoraro on 31 Jan 2017, 01:36 UTC:

Do you have a way of running all (or a large portion of) the tests? I'm afraid if I do it I won't be able to test enough of the tests to guarantee there will be no regressions.

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @gsnedders on 31 Jan 2017, 01:37 UTC:

I can, though it takes a while. Probably easier when I'm not travelling and using my laptop. :)

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @Ms2ger on 31 Jan 2017, 10:54 UTC:

I'm a little concerned about changing this; would using .bind(this) be sufficient?

@wpt-issue-mover
Copy link
Author

Originally posted as w3c/testharness.js#236 (comment) by @JosephPecoraro on 31 Jan 2017, 16:04 UTC:

bind would override the this that we are trying to test. Or were you thinking of using it on something else?

@wpt-issue-mover wpt-issue-mover mentioned this issue Aug 31, 2017
Closed
@domenic
Copy link
Member

domenic commented Sep 23, 2019

IMO this should be closed as people can use arrow functions now, or .bind(this) if necessary, to preserve the outer this.

@gsnedders
Copy link
Member

Yeah, I think arrow functions essentially make this a non-issue now. If anyone disagrees, comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
infra priority:backlog testharness.js
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bobholt @gsnedders @domenic @wpt-issue-mover