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

Asynchronous step function that does not modify this value #7182

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

Comments

@wpt-issue-mover
Copy link

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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

This comment has been minimized.

Copy link
Author

@wpt-issue-mover wpt-issue-mover commented Aug 31, 2017

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?

@domenic

This comment has been minimized.

Copy link
Member

@domenic 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

This comment has been minimized.

Copy link
Contributor

@gsnedders gsnedders commented Oct 10, 2019

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

@gsnedders gsnedders closed this Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.