mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-03 21:43:21 +00:00 
			
		
		
		
	Fix false positives in message view tests.
The `assert_message_groups_list_equal` and `assert_message_list_equal` helpers were always returning `true`, as they were doing a bogus traversal of the data structures and always comparing empty arrays, even when there was real data. To prevent this pitfall in the future, we assert that the extracted data is truth-y, and for the empty cases we just directly assert deep equality to `[]`.
This commit is contained in:
		@@ -90,22 +90,28 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        return list;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    function extract_message_ids(lst) {
 | 
			
		||||
        return _.map(lst, (item) => {
 | 
			
		||||
            return item.msg.id;
 | 
			
		||||
        });
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    function assert_message_list_equal(list1, list2) {
 | 
			
		||||
        assert.deepEqual(
 | 
			
		||||
            _.chain(list1).pluck('msg').pluck('id').value(),
 | 
			
		||||
            _.chain(list2).pluck('msg').pluck('id').value());
 | 
			
		||||
        var ids1 = extract_message_ids(list1);
 | 
			
		||||
        var ids2 = extract_message_ids(list2);
 | 
			
		||||
        assert(ids1.length);
 | 
			
		||||
        assert.deepEqual(ids1, ids2);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    function extract_group(group) {
 | 
			
		||||
        return extract_message_ids(group.message_containers);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    function assert_message_groups_list_equal(list1, list2) {
 | 
			
		||||
        function extract_message_ids(message_group) {
 | 
			
		||||
            return _.chain(message_group.messages)
 | 
			
		||||
                .pluck('msg')
 | 
			
		||||
                .pluck('id')
 | 
			
		||||
                .value();
 | 
			
		||||
        }
 | 
			
		||||
        assert.deepEqual(
 | 
			
		||||
            _.map(list1, extract_message_ids),
 | 
			
		||||
            _.map(list2, extract_message_ids));
 | 
			
		||||
        var ids1 = _.map(list1, extract_group);
 | 
			
		||||
        var ids2 = _.map(list2, extract_group);
 | 
			
		||||
        assert(ids1.length);
 | 
			
		||||
        assert.deepEqual(ids1, ids2);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    (function test_empty_list_bottom() {
 | 
			
		||||
@@ -118,10 +124,10 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
 | 
			
		||||
        assert_message_groups_list_equal(list._message_groups, [message_group]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, [message_group]);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_append_message_same_subject() {
 | 
			
		||||
@@ -142,9 +148,9 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        assert_message_groups_list_equal(
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [build_message_group([message1, message2])]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_groups, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, [message2]);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, [message1]);
 | 
			
		||||
    }());
 | 
			
		||||
@@ -169,10 +175,10 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [message_group1, message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, [message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_append_message_different_day() {
 | 
			
		||||
@@ -195,10 +201,10 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [message_group1, message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, [message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_append_message_historical() {
 | 
			
		||||
@@ -221,10 +227,10 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [message_group1, message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, [message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_append_message_same_subject_me_message() {
 | 
			
		||||
@@ -246,9 +252,9 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        assert_message_groups_list_equal(
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [build_message_group([message1, message2])]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_groups, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, [message2]);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, [message1]);
 | 
			
		||||
    }());
 | 
			
		||||
@@ -272,12 +278,12 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        assert_message_groups_list_equal(
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [build_message_group([message2, message1])]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_groups, []);
 | 
			
		||||
        assert.deepEqual(result.prepend_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups,
 | 
			
		||||
                                         [build_message_group([message2, message1])]);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_prepend_message_different_subject() {
 | 
			
		||||
@@ -298,11 +304,11 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        assert_message_groups_list_equal(
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [message_group2, message_group1]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, [message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_prepend_message_different_day() {
 | 
			
		||||
@@ -326,11 +332,11 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        assert_message_groups_list_equal(
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [message_group2, message_group1]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, [message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, [message_group1]);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
    (function test_prepend_message_historical() {
 | 
			
		||||
@@ -352,11 +358,11 @@ run_test('merge_message_groups', () => {
 | 
			
		||||
        assert_message_groups_list_equal(
 | 
			
		||||
            list._message_groups,
 | 
			
		||||
            [message_group2, message_group1]);
 | 
			
		||||
        assert_message_groups_list_equal(result.append_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_groups, []);
 | 
			
		||||
        assert_message_groups_list_equal(result.prepend_groups, [message_group2]);
 | 
			
		||||
        assert_message_groups_list_equal(result.rerender_groups, []);
 | 
			
		||||
        assert_message_list_equal(result.append_messages, []);
 | 
			
		||||
        assert_message_list_equal(result.rerender_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_groups, []);
 | 
			
		||||
        assert.deepEqual(result.append_messages, []);
 | 
			
		||||
        assert.deepEqual(result.rerender_messages, []);
 | 
			
		||||
    }());
 | 
			
		||||
 | 
			
		||||
});
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user