zjquery: Use Proxy to detect undefined stubs.

We now use a Proxy to wrap zjquery elements, so
that we can detect callers trying to invoke methods
(or access attributes) that do not exist.  We try
to give useful error messages in those cases.

The main impact here is that we force lots of tests
to explicitly stub `length`.

Also, we can't do equality checks on zjquery
objects any more due to the proxy object, but the
easy workaround is to compare selectors.  (This
is generally an unnecessary technique, anyway.)

The proxy wrapper is fairly straightforward, and
we just have a few special cases for things like
"inspect" that happen when you try to print out
objects.
This commit is contained in:
Steve Howell
2019-04-18 19:11:30 +00:00
committed by Tim Abbott
parent 9b32c844df
commit 6b39d6004e
7 changed files with 56 additions and 5 deletions

View File

@@ -74,9 +74,8 @@ the stub for a function you're calling in your patch. Typically the stub
is just placed in the test file, to prevent bloating of `zjquery`
with functions that are only used in a single test.
A good sign that you need to stub something out is getting an error of
the type:
`TypeError: <component>.<method> is not a function`
If you need to stub, you will see an error of this form:
`Error: You must create a stub for $("#foo").bar`
The `zjquery` library itself is only about 500 lines of code, and can
also be a useful resource if you're having trouble debugging DOM

View File

@@ -1579,7 +1579,7 @@ run_test('on_events', () => {
setup_mock_markdown_contains_backend_only_syntax('```foobarfoobar```', true);
setup_mock_markdown_is_status_message('```foobarfoobar```', 'Server: foobarfoobar', false);
loading.make_indicator = function (spinner) {
assert.equal(spinner, $("#markdown_preview_spinner"));
assert.equal(spinner.selector, "#markdown_preview_spinner");
make_indicator_called = true;
};
mock_channel_post('```foobarfoobar```');

View File

@@ -327,6 +327,8 @@ run_test('format_drafts', () => {
drafts.open_modal = noop;
drafts.set_initial_element = noop;
$("#drafts_table .draft-row").length = 0;
drafts.launch();
timerender.render_now = stub_render_now;
});

View File

@@ -213,6 +213,9 @@ run_test('update_dom_with_unread_counts', () => {
child_li.set_find_results('.private_message_count', child_count);
child_count.set_find_results('.value', child_value);
child_value.length = 1;
child_count.length = 1;
var pm_count = new Dict();
var user_ids_string = '101,102';
pm_count.set(user_ids_string, 7);

View File

@@ -119,6 +119,8 @@ run_test('create_sidebar_row', () => {
var social_li = $('<social sidebar row>');
var stream_id = social.stream_id;
social_li.length = 0;
var privacy_elem = $.create('privacy-stub');
social_li.set_find_results('.stream-privacy', privacy_elem);
@@ -567,6 +569,7 @@ run_test('update_count_in_dom', () => {
'<stream-value>'
);
$('<stream li>').length = 0;
stream_li.addClass('subscription_block');
stream_li.addClass('stream-with-count');
assert(stream_li.hasClass('stream-with-count'));
@@ -628,6 +631,8 @@ run_test('rename_stream', () => {
stream_data.rename_sub(sub, new_name);
const li_stub = $.create('li stub');
li_stub.length = 0;
templates.render = (name, payload) => {
assert.equal(name, 'stream_sidebar_row');
assert.deepEqual(payload, {
@@ -672,6 +677,8 @@ run_test('refresh_pin', () => {
});
const li_stub = $.create('li stub');
li_stub.length = 0;
templates.render = () => {
return {to_$: () => li_stub};
};

View File

@@ -160,6 +160,7 @@ const count_stub = $.create('count');
count_stub.set_find_results('.value', value_stub);
$(".top_left_starred_messages").set_find_results('.count', count_stub);
$("#tab_list .stream").length = 0;
run_test('initialize_everything', () => {
ui_init.initialize_everything();

View File

@@ -377,7 +377,46 @@ exports.make_zjquery = function (opts) {
silent: opts.silent,
});
add_extensions(elem);
return elem;
// Create a proxy handler to detect missing stubs.
//
// For context, zjquery doesn't implement every method/attribute
// that you'd find on a "real" jQuery object. Sometimes we
// expects devs to create their own stubs.
var handler = {
get: (target, key) => {
// Handle the special case of equality checks, which
// we can infer by assert.equal trying to access the
// "stack" key.
if (key === 'stack') {
var error = '\nInstead of doing equality checks on a full object, ' +
'do `assert_equal(foo.selector, ".some_class")\n';
throw Error(error);
}
const val = target[key];
if (val === undefined) {
// For undefined values, we'll throw errors to devs saying
// they need to create stubs. We ignore certain keys that
// are used for simply printing out the object.
if (typeof key === 'symbol') {
return;
}
if (key === 'inspect') {
return;
}
throw Error('You must create a stub for $("' + selector + '").' + key);
}
return val;
},
};
var proxy = new Proxy(elem, handler);
return proxy;
}
var zjquery = function (arg, arg2) {