Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for /_matrix/media/r0/config #456

Closed
wants to merge 1 commit into from
Closed

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Jun 22, 2018

Not sure how I will go about testing the value, or if it's necessary.

Tests matrix-org/synapse#3184

@Half-Shot Half-Shot requested a review from a team July 9, 2018 13:34
@richvdh
Copy link
Member

richvdh commented Jul 17, 2018

if you made your branches have the same name, the sytests would run against the right branch of synapse...

)->then( sub {
my ( $body, $response ) = @_;

assert_json_keys( $body, qw( m.upload.size ) );
Copy link
Member

Choose a reason for hiding this comment

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

given that this is optional, I'm not sure it is correct to assert it here.

You could assert that, if it is present, it is an int of a plausible size?

@Half-Shot
Copy link
Contributor Author

if you made your branches have the same name, the sytests would run against the right branch of synapse...

This really aught to be mentioned somewhere on the matrix-org/synapse README.

@richvdh
Copy link
Member

richvdh commented Jul 18, 2018

aught

You keep using that word. I do not think it means what you think it means.

Anyway, if anywhere, it should be in CONTRIBUTING, but it's a hack that only works for people in the matrix-org github org, so I'm unsure it should be there.

@Half-Shot
Copy link
Contributor Author

Closing and reopening so sytest will apparently test against the correct thing.

@Half-Shot Half-Shot closed this Jul 18, 2018
@Half-Shot Half-Shot mentioned this pull request Jul 18, 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.

2 participants