From 2cd810917904ddf6b612375298e7d8c63a8926fc Mon Sep 17 00:00:00 2001 From: Andrew Mao Date: Mon, 1 Aug 2016 20:25:43 -0400 Subject: [PATCH 1/2] Add Turkserver.treatment on server --- package.js | 1 + server/server_api.js | 19 +++++++++++++++++++ tests/experiment_tests.coffee | 2 ++ 3 files changed, 22 insertions(+) create mode 100644 server/server_api.js diff --git a/package.js b/package.js index a2fd5f2..83f9c2f 100644 --- a/package.js +++ b/package.js @@ -74,6 +74,7 @@ Package.onUse(function (api) { api.addFiles([ 'server/config.js', 'server/turkserver.coffee', + 'server/server_api.js', 'server/mturk.js', 'server/lobby_server.coffee', 'server/batches.coffee', diff --git a/server/server_api.js b/server/server_api.js new file mode 100644 index 0000000..2e8d792 --- /dev/null +++ b/server/server_api.js @@ -0,0 +1,19 @@ +/** + * @summary Access treatment data assigned to the current user (assignment) + * or the user's current world (instance). + * @locus Server + * @returns {Object} treatment key/value pairs + */ +TurkServer.treatment = function() { + const instance = TurkServer.Instance.currentInstance(); + const asst = TurkServer.Assignment.currentAssignment(); + + const instTreatments = instance && instance.getTreatmentNames() || []; + const asstTreatments = asst && asst.getTreatmentNames() || []; + + return TurkServer._mergeTreatments(Treatments.find({ + name: { + $in: instTreatments.concat(asstTreatments) + } + })); +}; diff --git a/tests/experiment_tests.coffee b/tests/experiment_tests.coffee index f0279ff..0a10454 100644 --- a/tests/experiment_tests.coffee +++ b/tests/experiment_tests.coffee @@ -149,6 +149,8 @@ Tinytest.add "experiment - instance - teardown and log", withCleanup (test) -> Tinytest.add "experiment - instance - get treatment on server", withCleanup (test) -> instance = batch.createInstance(["fooTreatment"]) + # Note this only tests world treatments. Assignment treatments have to be + # tested with the janky client setup. instance.bindOperation -> treatment = TurkServer.treatment() test.equal treatment.treatments[0], "fooTreatment" From 24144aa965d2738985c44cbd4087ab7d421cea87 Mon Sep 17 00:00:00 2001 From: Andrew Mao Date: Fri, 12 May 2017 16:35:26 -0400 Subject: [PATCH 2/2] add tests, fix bugs, and reorganize some tests --- server/assignment.js | 15 ++++++++++-- server/instance.js | 10 -------- tests/experiment_client_tests.coffee | 34 ++++++++++++++++++++++++---- tests/experiment_tests.coffee | 34 +++++++++++++++++----------- 4 files changed, 63 insertions(+), 30 deletions(-) diff --git a/server/assignment.js b/server/assignment.js index 9754347..6579b39 100644 --- a/server/assignment.js +++ b/server/assignment.js @@ -86,8 +86,19 @@ class Assignment { * @returns {TurkServer.Assignment} The assignment object. */ static currentAssignment() { - let userId = Meteor.userId(); - if (userId == null) return; + let userId = null; + try { + userId = Meteor.userId(); + } + catch (e) { + // We aren't in a method, so Meteor throws this error: + // "Error: Meteor.userId can only be invoked in method calls. Use this.userId in publish functions." + // Note that isn't just publish functions, but any server code not + // triggered by a client. + return null; + } + if (userId == null) return null; + return Assignment.getCurrentUserAssignment(userId); } diff --git a/server/instance.js b/server/instance.js index 38c0e5e..67464b9 100644 --- a/server/instance.js +++ b/server/instance.js @@ -4,16 +4,6 @@ const init_queue = []; XXX Note that the collection called "Experiments" now actually refers to instances */ -/** - * @summary Get the treatments of the current instance - * @locus Anywhere - * @returns The treatments of the currently scoped instance - */ -TurkServer.treatment = () => { - const inst = TurkServer.Instance.currentInstance(); - return inst && inst.treatment(); -}; - // map of groupId to instance objects // XXX Can't use WeakMap here because we have primitive keys const _instances = new Map(); diff --git a/tests/experiment_client_tests.coffee b/tests/experiment_client_tests.coffee index 256a595..14491b1 100644 --- a/tests/experiment_client_tests.coffee +++ b/tests/experiment_client_tests.coffee @@ -58,10 +58,22 @@ if Meteor.isServer TurkServer.Instance.currentInstance().teardown(returnToLobby) return + getServerTreatment: -> + return TurkServer.treatment() + if Meteor.isClient tol = 20 # range in ms that we can be off in adjacent cols big_tol = 500 # max range we tolerate in a round trip to the server (async method) + expectedTreatment = { + fooProperty: "bar" # world + foo2: "baz" # user + } + + checkTreatments = (test, obj) -> + for k, v of expectedTreatment + test.equal obj[k], v, "for key #{k} actual value #{obj[k]} doesn't match expected value #{v}" + Tinytest.addAsync "experiment - client - login and creation of assignment metadata", (test, next) -> InsecureLogin.ready -> test.isTrue Meteor.userId() @@ -107,6 +119,8 @@ if Meteor.isClient test.isTrue userTreatment.name test.equal userTreatment.foo2, "baz" + checkTreatments(test, TurkServer.treatment()) + next() fail = -> @@ -119,6 +133,15 @@ if Meteor.isClient return true if treatment.treatments.length ), verify, fail, 2000 + Tinytest.addAsync "experiment - assignment - test treatments on server", (test, next) -> + # Even though this is a "client" test, it is testing a server function + # because assignment treatments are different on the client and server + Meteor.call "getServerTreatment", (err, res) -> + test.fail() if err? + + checkTreatments(test, res) + next() + Tinytest.addAsync "experiment - client - current payment variable", (test, next) -> amount = 0.42 @@ -126,7 +149,7 @@ if Meteor.isClient test.equal TurkServer.currentPayment(), amount next() - Tinytest.addAsync "experiment - client - assignment metadata and local time vars", (test, next) -> + Tinytest.addAsync "experiment - assignment - assignment metadata and local time vars", (test, next) -> asstData = null verify = -> @@ -153,7 +176,7 @@ if Meteor.isClient return true if asstData? ), verify, fail, 2000 - Tinytest.addAsync "experiment - client - no time fields", (test, next) -> + Tinytest.addAsync "experiment - assignment - no time fields", (test, next) -> fields = [ { id: TurkServer.group() @@ -181,7 +204,7 @@ if Meteor.isClient next() - Tinytest.addAsync "experiment - client - joined time computation", (test, next) -> + Tinytest.addAsync "experiment - assignment - joined time computation", (test, next) -> fields = [ { id: TurkServer.group() @@ -211,7 +234,7 @@ if Meteor.isClient next() - Tinytest.addAsync "experiment - client - instance ended state", (test, next) -> + Tinytest.addAsync "experiment - instance - instance ended state", (test, next) -> # In experiment. not ended test.isTrue TurkServer.inExperiment() test.isFalse TurkServer.instanceEnded() @@ -226,7 +249,8 @@ if Meteor.isClient Next test edits instance fields, so client APIs may break state ### - Tinytest.addAsync "experiment - client - selects correct instance of multiple", (test, next) -> + Tinytest.addAsync "experiment - instance - client selects correct instance of + multiple", (test, next) -> fields = [ { id: Random.id() diff --git a/tests/experiment_tests.coffee b/tests/experiment_tests.coffee index 0a10454..1fdfd25 100644 --- a/tests/experiment_tests.coffee +++ b/tests/experiment_tests.coffee @@ -79,6 +79,10 @@ Tinytest.add "experiment - batch - creation and retrieval", withCleanup (test) - test.equal batch2, batch +Tinytest.add "experiment - assignment - currentAssignment in standalone + server code returns null", (test) -> + test.equal TurkServer.Assignment.currentAssignment(), null + Tinytest.add "experiment - instance - throws error if doesn't exist", withCleanup (test) -> test.throws -> TurkServer.Instance.getInstance("yabbadabbadoober") @@ -151,12 +155,16 @@ Tinytest.add "experiment - instance - get treatment on server", withCleanup (tes # Note this only tests world treatments. Assignment treatments have to be # tested with the janky client setup. + + # However, This also tests accessing server treatments outside of a client context. instance.bindOperation -> treatment = TurkServer.treatment() test.equal treatment.treatments[0], "fooTreatment" + test.equal treatment.fooProperty, "bar" # Undefined outside of an experiment instance - test.equal TurkServer.treatment(), undefined + treatment = TurkServer.treatment() + test.equal treatment.fooProperty, undefined Tinytest.add "experiment - instance - global group", withCleanup (test) -> instance = batch.createInstance([]) @@ -183,7 +191,7 @@ Tinytest.add "experiment - instance - global group", withCleanup (test) -> test.equal stuff[2].foo2, "bar" test.equal stuff[2]._groupId, instance.groupId -Tinytest.add "experiment - instance - reject adding user to ended instance", withCleanup (test) -> +Tinytest.add "experiment - assignment - reject adding user to ended instance", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -203,7 +211,7 @@ Tinytest.add "experiment - instance - reject adding user to ended instance", wit test.isFalse asstData.instances -Tinytest.add "experiment - instance - addAssignment records start time and instance id", withCleanup (test) -> +Tinytest.add "experiment - assignment - addAssignment records start time and instance id", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -227,7 +235,7 @@ Tinytest.add "experiment - instance - addAssignment records start time and insta test.equal asstData.instances[0].id, instance.groupId test.isTrue asstData.instances[0].joinTime -Tinytest.add "experiment - instance - second addAssignment does not change date", withCleanup (test) -> +Tinytest.add "experiment - assignment - second addAssignment does not change date", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -249,7 +257,7 @@ Tinytest.add "experiment - instance - second addAssignment does not change date" # Should be the same date as originally test.equal instanceData.startTime, startedDate -Tinytest.add "experiment - instance - teardown with returned assignment", withCleanup (test) -> +Tinytest.add "experiment - assignment - teardown with returned assignment", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -269,7 +277,7 @@ Tinytest.add "experiment - instance - teardown with returned assignment", withCl test.isTrue asstData.instances[0] test.equal asstData.status, "returned" -Tinytest.add "experiment - instance - user disconnect and reconnect", withCleanup (test) -> +Tinytest.add "experiment - assignment - user disconnect and reconnect", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -308,7 +316,7 @@ Tinytest.add "experiment - instance - user disconnect and reconnect", withCleanu test.isTrue asstData.instances[0].disconnectedTime > 0 test.isTrue asstData.instances[0].disconnectedTime < Date.now() - discTime -Tinytest.add "experiment - instance - user idle and re-activate", withCleanup (test) -> +Tinytest.add "experiment - assignment - user idle and re-activate", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -364,7 +372,7 @@ Tinytest.add "experiment - instance - user idle and re-activate", withCleanup (t test.isFalse asstData.instances[0].lastIdle test.equal asstData.instances[0].idleTime, offset + offset -Tinytest.add "experiment - instance - user disconnect while idle", withCleanup (test) -> +Tinytest.add "experiment - assignment - user disconnect while idle", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -389,7 +397,7 @@ Tinytest.add "experiment - instance - user disconnect while idle", withCleanup ( # Check that disconnect fields exist test.isTrue asstData.instances[0].lastDisconnect -Tinytest.add "experiment - instance - idleness is cleared on reconnection", withCleanup (test) -> +Tinytest.add "experiment - assignment - idleness is cleared on reconnection", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -421,7 +429,7 @@ Tinytest.add "experiment - instance - idleness is cleared on reconnection", with test.isFalse asstData.instances[0].lastDisconnect test.isTrue asstData.instances[0].disconnectedTime -Tinytest.add "experiment - instance - teardown while disconnected", withCleanup (test) -> +Tinytest.add "experiment - assignment - teardown while disconnected", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -448,7 +456,7 @@ Tinytest.add "experiment - instance - teardown while disconnected", withCleanup test.isTrue asstData.instances[0].disconnectedTime > 0 test.isTrue asstData.instances[0].disconnectedTime < Date.now() - discTime -Tinytest.add "experiment - instance - teardown while idle", withCleanup (test) -> +Tinytest.add "experiment - assignment - teardown while idle", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -472,7 +480,7 @@ Tinytest.add "experiment - instance - teardown while idle", withCleanup (test) - test.isFalse asstData.instances[0].lastIdle test.isTrue asstData.instances[0].idleTime -Tinytest.add "experiment - instance - leave instance after teardown", withCleanup (test) -> +Tinytest.add "experiment - assignment - leave instance after teardown", withCleanup (test) -> instance = batch.createInstance([]) instance.setup() @@ -503,7 +511,7 @@ Tinytest.add "experiment - instance - leave instance after teardown", withCleanu test.isTrue asstData.instances[0].disconnectedTime > 0 test.isTrue asstData.instances[0].disconnectedTime < 200 -Tinytest.add "experiment - instance - teardown and join second instance", withCleanup (test) -> +Tinytest.add "experiment - assignment - teardown and join second instance", withCleanup (test) -> instance = batch.createInstance([]) instance.setup()