Skip to content

Commit

Permalink
base1: Drop jQuery usage from test-chan
Browse files Browse the repository at this point in the history 
This test previously failed with the standard JS EventListener API, as
with that the registered listeners get called asynchronously, while with
jQuery they got called synchronously. This caused send() to _first_ call
the fallback MockPeer.onrecv() echo implementation, and _then_ the
registered "real" recv handler. Thus the former sent wrong and
unexpected replies. To fix that, postpone onrecv()'s own reply sending,
so that the event listeners have a chance to run first.

Adjust the number of expected messages in "filter message in" for that,
as this now does not see MockPeer.onrecv()'s standard reply any more.

This was the last jQuery consumer in the unit tests (and all of base1),
so drop the jquery bundling hack as well.

Closes cockpit-project#14918
  • Loading branch information
@martinpitt
martinpitt committed Nov 19, 2020
1 parent 93307fd commit 66e283f
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 38 deletions.
7 changes: 0 additions & 7 deletions src/base1/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,6 @@ $(base_TESTS): dist/base1/test-%.html: src/base1/test-%.js $(base_DEPS)
cp -L $< $@.tmp.js && $(MV) $@.tmp.js $(dir $@)/$(notdir $<) && \
$(SED_TEST_TEMPLATE) $(srcdir)/src/base1/qunit-template.html > $@.tmp && $(MV) $@.tmp $@

# HACK: last user of jquery is test-chan; port that away and drop this special-case rule
dist/base1/test-chan.html: src/base1/test-chan.js $(base_QUNIT_DEPS)
$(ESLINT_RULE)
$(AM_V_GEN) $(MKDIR_P) $(dir $@) && \
cat $(srcdir)/node_modules/jquery/dist/jquery.slim.min.js $< > $@.tmp.js && $(MV) $@.tmp.js $(dir $@)/$(notdir $<) && \
$(SED_TEST_TEMPLATE) $(srcdir)/src/base1/qunit-template.html > $@.tmp && $(MV) $@.tmp $@

# Simple tests run without QUnit

dist/base1/test-%.js: dist/base1/test-%.html
Expand Down
65 changes: 34 additions & 31 deletions src/base1/test-chan.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global $, cockpit, QUnit */
/* global cockpit, QUnit */

/* Set this to a regexp to ignore that warning once */
function console_ignore_log(exp) {
Expand Down Expand Up @@ -33,25 +33,28 @@ function MockPeer() {

command = JSON.parse(payload);
if (command.command == "open") {
let reply;
if (command.payload == "echo") {
echos[command.channel] = true;
self.send(null, JSON.stringify({
reply = {
command: "ready",
channel: command.channel
}));
};
} else if (command.payload == "null") {
nulls[command.channel] = true;
self.send(null, JSON.stringify({
reply = {
command: "ready",
channel: command.channel
}));
};
} else {
self.send(null, JSON.stringify({
reply = {
command: "close",
channel: command.channel,
problem: "not-supported",
}));
};
}
// HACK: send the reply asynchronously, so that mock_peer addEventListener() gets to see it before our own onrecv()
window.setTimeout(() => self.send(null, JSON.stringify(reply), 0));
} else if (command.command == "close") {
delete echos[command.channel];
delete nulls[command.channel];
Expand Down Expand Up @@ -100,7 +103,7 @@ function MockWebSocket(url, protocol) {
throw Error("Invalid frame sent to WebSocket: " + data);
var channel = data.substring(0, pos);
var payload = data.substring(pos + 1);
window.setTimeout(function() { $(mock).triggerHandler("recv", [channel, payload]) }, 5);
window.setTimeout(function() { mock.dispatchEvent("recv", channel, payload) }, 5);
};

this.close = function(code, reason) {
Expand Down Expand Up @@ -231,7 +234,7 @@ QUnit.test("open channel", function (assert) {
mock_peer.addEventListener("open", function(event) {
assert.ok(true, "websocket connected");
});
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
var command = JSON.parse(payload);
if (!is_inited) {
assert.equal(typeof command, "object", "valid json");
Expand All @@ -256,11 +259,11 @@ QUnit.test("multiple", function (assert) {
var channelb = cockpit.channel({ host: "amy" });

const onrecv = event => {
$(mock_peer).off("recv");
mock_peer.removeEventListener("recv", onrecv);
assert.notStrictEqual(channel.id, channelb.id, "channels have different ids");
done();
};
$(mock_peer).on("recv", onrecv);
mock_peer.addEventListener("recv", onrecv);
});

QUnit.test("open no host", function (assert) {
Expand All @@ -272,7 +275,7 @@ QUnit.test("open no host", function (assert) {
mock_peer.addEventListener("open", function(event) {
assert.ok(true, "websocket connected");
});
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
var command = JSON.parse(payload);
if (command.command == "open") {
assert.strictEqual(command.host, undefined, "host not included");
Expand All @@ -291,7 +294,7 @@ QUnit.test("open auto host", function (assert) {
mock_peer.addEventListener("open", function(event) {
assert.ok(true, "websocket connected");
});
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
var command = JSON.parse(payload);
if (command.command == "open") {
assert.strictEqual(command.host, "planetexpress", "host automatically chosen");
Expand All @@ -308,7 +311,7 @@ QUnit.test("send message", function (assert) {
mock_peer.addEventListener("open", function(event) {
channel.send("Scruffy gonna die the way he lived");
});
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
/* Ignore the open and init messages */
if (!chan)
return;
Expand All @@ -328,7 +331,7 @@ QUnit.test("queue messages", function (assert) {
channel.send("knows");
channel.send("he");
channel.send("rules");
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
if (!chan)
return; /* ignore control messages */
sentence.push(payload);
Expand All @@ -343,14 +346,14 @@ QUnit.test("receive message", function (assert) {
const done = assert.async();
assert.expect(1);

function onrecv(event, chan, payload) {
const onrecv = (event, chan, payload) => {
const cmd = JSON.parse(payload);
if (cmd.command == "open") {
$(mock_peer).off("recv");
mock_peer.removeEventListener("recv", onrecv);
mock_peer.send(channel.id, "Oh, marrrrmalade!");
}
}
$(mock_peer).on("recv", onrecv);
};
mock_peer.addEventListener("recv", onrecv);

var channel = cockpit.channel({ });
channel.addEventListener("message", function(event, message) {
Expand All @@ -363,7 +366,7 @@ QUnit.test("close channel", function (assert) {
const done = assert.async(2);
assert.expect(4);

$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
const cmd = JSON.parse(payload);
if (cmd.command == "init") {
return;
Expand Down Expand Up @@ -402,7 +405,7 @@ QUnit.test("close problem", function (assert) {
const done = assert.async();
assert.expect(5);

$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
const cmd = JSON.parse(payload);
if (cmd.command == "init") {
return;
Expand All @@ -427,7 +430,7 @@ QUnit.test("close problem string", function (assert) {
assert.expect(5);

var channel = cockpit.channel({ });
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
const cmd = JSON.parse(payload);
if (cmd.command == "init") {
return;
Expand All @@ -450,7 +453,7 @@ QUnit.test("close peer", function (assert) {
const done = assert.async();
assert.expect(5);

$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
var msg = JSON.parse(payload);
if (msg.command == "init")
return;
Expand Down Expand Up @@ -557,7 +560,7 @@ QUnit.test("wait callback", function (assert) {

QUnit.test("logout", function (assert) {
const done = assert.async();
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
const cmd = JSON.parse(payload);
if (cmd.command == "logout") {
mock_peer.close("disconnected");
Expand Down Expand Up @@ -590,7 +593,7 @@ QUnit.test("logout", function (assert) {
QUnit.test("droppriv", function (assert) {
const done = assert.async();
assert.expect(1);
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
const cmd = JSON.parse(payload);
if (cmd.command == "logout") {
assert.strictEqual(cmd.disconnect, false, "disconnect not set");
Expand All @@ -617,13 +620,13 @@ QUnit.test("info", function (assert) {
const onrecv = (event, chan, payload) => {
const cmd = JSON.parse(payload);
if (cmd.command == "open") {
$(mock_peer).off("recv");
mock_peer.removeEventListener("recv", onrecv);
cockpit.info.removeEventListener("changed", onchanged);
assert.strictEqual(info_changed, true, "info changed event was called");
done();
}
};
$(mock_peer).on("recv", onrecv);
mock_peer.addEventListener("recv", onrecv);

var channel = cockpit.channel({ host: "scruffy" });
assert.ok(channel);
Expand All @@ -637,7 +640,7 @@ QUnit.test("send after close", function (assert) {

var received_message = false;
var channel = cockpit.channel({ });
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
if (chan)
received_message = true;
});
Expand Down Expand Up @@ -670,7 +673,7 @@ QUnit.test("ignore other commands", function (assert) {

QUnit.test("filter message in", function (assert) {
const done = assert.async();
assert.expect(14);
assert.expect(11);

var filtered = 0;
var filtering = true;
Expand Down Expand Up @@ -752,7 +755,7 @@ QUnit.test("inject message out", function (assert) {

var first = true;
var channel;
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
if (first) {
var ret = cockpit.transport.inject("bree\nyellow");
assert.equal(ret, true, "returned true");
Expand Down Expand Up @@ -797,7 +800,7 @@ QUnit.test("transport options", function (assert) {
assert.expect(3);

var channel;
$(mock_peer).on("recv", function(event, chan, payload) {
mock_peer.addEventListener("recv", function(event, chan, payload) {
if (chan) {
assert.equal(typeof cockpit.transport.options, "object", "is an object");
assert.deepEqual(cockpit.transport.options, {
Expand Down

0 comments on commit 66e283f

Please sign in to comment.