node tests: Always enforce blueslip warn/error/fatal.

We now require all of our unit tests to handle
blueslip errors for warn/error/fatal.  This
simplifies the zblueslip code to not have any
options passed in.

Most of the places changed here fell into two
categories:

    - We were just missing a random piece of
      setup data in a happy path test.

    - We were testing error handling in just
      a lazy way to ensure 100% coverage.  Often
      these error codepaths were fairly
      contrived.

The one place where we especially lazy was
the stream_data tests, and those are now
more thorough.
This commit is contained in:
Steve Howell
2020-02-07 21:26:05 +00:00
committed by Tim Abbott
parent 996d054fe9
commit e9c6653852
8 changed files with 26 additions and 31 deletions

View File

@@ -1,6 +1,6 @@
set_global('bridge', false);
set_global('blueslip', global.make_zblueslip({}));
set_global('blueslip', global.make_zblueslip());
const noop = function () {};

View File

@@ -1,9 +1,7 @@
zrequire('util');
zrequire('people');
set_global('blueslip', global.make_zblueslip({
error: false, // We check for errors in people_errors.js
}));
set_global('blueslip', global.make_zblueslip());
set_global('message_store', {});
set_global('page_params', {});
set_global('settings_org', {});
@@ -154,7 +152,6 @@ run_test('my_custom_profile_data', () => {
person.profile_data = {3: 'My address', 4: 'My phone number'};
assert.equal(people.my_custom_profile_data(3), 'My address');
assert.equal(people.my_custom_profile_data(4), 'My phone number');
assert.equal(people.my_custom_profile_data(undefined), undefined);
});
run_test('bot_custom_profile_data', () => {
@@ -198,6 +195,7 @@ run_test('user_timezone', () => {
run_test('user_type', () => {
const realm_admin = {
email: 'realm_admin@example.com',
full_name: 'Realm Admin',
user_id: 32,
is_admin: true,
is_guest: false,
@@ -205,6 +203,7 @@ run_test('user_type', () => {
};
const guest = {
email: 'guest@example.com',
full_name: 'Guest User',
user_id: 33,
is_admin: false,
is_guest: true,
@@ -212,6 +211,7 @@ run_test('user_type', () => {
};
const bot = {
email: 'bot@example.com',
full_name: 'Example Bot',
user_id: 34,
is_admin: false,
is_guest: false,
@@ -267,8 +267,6 @@ run_test('set_custom_profile_field_data', () => {
const person = people.get_by_email(me.email);
me.profile_data = {};
const field = {id: 3, name: 'Custom long field', type: 'text', value: 'Field value', rendered_value: '<p>Field value</p>'};
people.set_custom_profile_field_data(person.user_id, {});
assert.deepEqual(person.profile_data, {});
people.set_custom_profile_field_data(person.user_id, field);
assert.equal(person.profile_data[field.id].value, 'Field value');
assert.equal(person.profile_data[field.id].rendered_value, '<p>Field value</p>');
@@ -311,7 +309,7 @@ initialize();
run_test('recipient_counts', () => {
const user_id = 99;
assert.equal(people.get_recipient_count({id: user_id}), 0);
assert.equal(people.get_recipient_count({user_id: user_id}), 0);
people.incr_recipient_count(user_id);
people.incr_recipient_count(user_id);
assert.equal(people.get_recipient_count({user_id: user_id}), 2);
@@ -499,6 +497,7 @@ run_test('message_methods', () => {
'https://secure.gravatar.com/avatar/md5-athens@example.com?d=identicon&s=50'
);
blueslip.set_test_data('error', 'Unknown user_id in get_by_user_id: 9999999');
message = {
avatar_url: undefined,
sender_email: 'foo@example.com',

View File

@@ -6,9 +6,7 @@ set_global('reload_state', {
is_in_progress: return_false,
});
set_global('blueslip', global.make_zblueslip({
debug: true, // testing for debug is disabled by default.
}));
set_global('blueslip', global.make_zblueslip());
const me = {
email: 'me@example.com',
@@ -134,4 +132,10 @@ run_test('blueslip', () => {
assert.equal(uri.indexOf('unk'), uri.length - 3);
assert.equal(blueslip.get_test_logs('error').length, 1);
blueslip.clear_test_data();
blueslip.set_test_data('error', 'Undefined field id');
assert.equal(people.my_custom_profile_data(undefined), undefined);
blueslip.set_test_data('error', 'Trying to set undefined field id');
people.set_custom_profile_field_data(maria.user_id, {});
});

View File

@@ -245,13 +245,8 @@ run_test('subscribers', () => {
assert(!ok);
assert.equal(blueslip.get_test_logs('warn').length, 2);
// Defensive code will give warnings, which we ignore for the
// tests, but the defensive code needs to not actually blow up.
set_global('blueslip', global.make_zblueslip({
warn: false,
}));
// verify that removing an already-removed subscriber is a noop
blueslip.set_test_data('warn', 'We tried to remove invalid subscriber: 104');
ok = stream_data.remove_subscriber('Rome', brutus.user_id);
assert(!ok);
assert(!stream_data.is_user_subscribed('Rome', brutus.user_id));
@@ -278,6 +273,9 @@ run_test('subscribers', () => {
stream_data.add_subscriber('Rome', brutus.user_id);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), true);
blueslip.set_test_data(
'warn',
'We got a is_user_subscribed call for a non-existent or inaccessible stream.');
sub.invite_only = true;
stream_data.update_calculated_fields(sub);
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), undefined);
@@ -285,6 +283,9 @@ run_test('subscribers', () => {
assert.equal(stream_data.is_user_subscribed('Rome', brutus.user_id), undefined);
// Verify that we don't crash and return false for a bad stream.
blueslip.set_test_data(
'warn',
'We got an add_subscriber call for a non-existent stream.');
ok = stream_data.add_subscriber('UNKNOWN', brutus.user_id);
assert(!ok);
@@ -564,8 +565,6 @@ run_test('delete_sub', () => {
assert(!stream_data.get_sub('Canada'));
assert(!stream_data.get_sub_by_id(canada.stream_id));
// We had earlier disabled warnings, so we need to remake zblueslip.
set_global('blueslip', global.make_zblueslip());
blueslip.set_test_data('warn', 'Failed to delete stream 99999');
stream_data.delete_sub(99999);
assert.equal(blueslip.get_test_logs('warn').length, 1);

View File

@@ -1,5 +1,5 @@
set_global('$', global.make_zjquery());
set_global('blueslip', global.make_zblueslip({}));
set_global('blueslip', global.make_zblueslip());
set_global('document', {});
zrequire('util');

View File

@@ -25,11 +25,6 @@ look at `node_tests/people_errors.js` for actual usage of this module.
// zblueslip stub variable.
set_global('blueslip', global.make_zblueslip());
// Aditionally, you can specify which functions you want to test for/ignore.
// By default, we ignore debug, log and info. To test for debug, for example, do:
// set_global('blueslip', global.make_zblueslip({debug: true}));
// Similarly, you can ignore tests for errors by passing {debug: true, error: false}.
run_test('basics', () => {
// Let's create a sample piece of code to test:
function throw_an_error() {

View File

@@ -1,9 +1,7 @@
exports.make_zblueslip = function (opts) {
exports.make_zblueslip = function () {
const lib = {};
// Apply defaults
opts = Object.assign({
const opts = {
// Silently swallow all debug, log and info calls.
debug: false,
log: false,
@@ -12,7 +10,7 @@ exports.make_zblueslip = function (opts) {
warn: true,
error: true,
fatal: true,
}, opts);
};
// Store valid test data for options.
lib.test_data = {};

View File

@@ -1119,7 +1119,7 @@ exports.set_full_name = function (person_obj, new_full_name) {
exports.set_custom_profile_field_data = function (user_id, field) {
if (field.id === undefined) {
blueslip.error("Unknown field id " + field.id);
blueslip.error("Trying to set undefined field id");
return;
}
people_by_user_id_dict.get(user_id).profile_data[field.id] = {