Skip to content

Tests for e2e key backup interface#503

Merged
dbkr merged 11 commits into
developfrom
dbkr/e2e_backups
Oct 15, 2018
Merged

Tests for e2e key backup interface#503
dbkr merged 11 commits into
developfrom
dbkr/e2e_backups

Conversation

@dbkr
Copy link
Copy Markdown
Member

@dbkr dbkr commented Oct 5, 2018

No description provided.

Copy link
Copy Markdown
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wooo, tests! Sorry that I'm not a fan of interdependent tests :(

Comment thread tests/41end-to-end-keys/07-backup.pl Outdated

assert_json_keys( $content, "is_verified" );

assert_json_keys( $content, "session_data" );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do:

assert_json_keys( $content, qw( first_message_index forwarded_count is_verified session_data ) )

Comment thread tests/41end-to-end-keys/07-backup.pl
Comment thread tests/41end-to-end-keys/07-backup.pl Outdated
is_verified => JSON::false,
session_data => "anopaquestring",
}
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird indentation

Comment thread tests/41end-to-end-keys/07-backup.pl Outdated
content => {
algorithm => "m.megolm_backup.v1",
auth_data => "anopaquestring",
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its going to be beneficial if we create matrix_create_backup_version etc functions. There's already a fair bit of duplication and it'll make it easier to write more backup tests.

@@ -0,0 +1,387 @@
my $fixture = local_user_fixture();

my $current_version; # FIXME: is there a better way of passing the backup version between tests?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if each test was a standalone test, rather than there being implicit dependencies (it makes it harder to reason about each test and change later on).

I think once the APIs are split out into separate functions that will reduce boiler plate enough to make it feasible to just start from scratch for each test. There are also multi_test if you want to do multiple tests in one run through.

Comment thread tests/41end-to-end-keys/07-backup.pl Outdated
assert_json_keys( $content, "session_data" );

$content->{first_message_index} == 1 or
die "Expected first message index to match submitted data";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to use assert_eq( $content->{first_message_index}, 1, ".." ) as it will have better logging on failures (in terms of logging what the actual values were?)

@dbkr
Copy link
Copy Markdown
Member Author

dbkr commented Oct 12, 2018

This should be better! ptal :)

@dbkr dbkr assigned erikjohnston and unassigned dbkr Oct 12, 2018
@dbkr dbkr merged commit f69c17e into develop Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants