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 a user with sufficient power level removing an alias. #818

Merged
merged 4 commits into from
Mar 4, 2020

Conversation

clokep
Copy link
Member

@clokep clokep commented Feb 25, 2020

SyTests for the behavior in matrix-org/synapse#6986.

A user should be able to delete an alias if they have sufficient power level (enough to modify the canonical alias event). There's already a separate test for a user not being able to delete aliases if they don't have power levels.

I verified that against a develop of master (i.e without matrix-org/synapse#6986) the added test here fails.

@clokep clokep requested a review from a team February 25, 2020 15:52
@clokep
Copy link
Member Author

clokep commented Feb 25, 2020

Oh, hmm...this is probably going to fail on Dendrite. How do we deal with features being added to Synapse before they're ported to Dendrite?

@clokep
Copy link
Member Author

clokep commented Feb 25, 2020

Looks like the dendrite build passed, so never mind. 👍

@anoadragon453
Copy link
Member

Looks like the dendrite build passed, so never mind. +1

I'm afraid it's quite confusing. Dendrite is currently marked as 'soft fail', so if it fails it doesn't prevent merging. On github you see a tick, but if you go to [buildkite](https://buildkite.com/matrix-dot-org/sytest/builds/608#b6f655d4-6a92-447e-8ef0-483beee7373e you'll see it did in fact fail.

tests/30rooms/05aliases.pl Outdated Show resolved Hide resolved
tests/30rooms/05aliases.pl Show resolved Hide resolved
tests/30rooms/05aliases.pl Outdated Show resolved Hide resolved
@clokep clokep requested a review from richvdh February 27, 2020 20:42
@richvdh
Copy link
Member

richvdh commented Feb 27, 2020

Having a final Future->done(1); seems to be common in this file.

For reference: each then sub needs to return a (successful) Future. So it's normal for the final then block to do its final checks and then return Future->done (ie, an already-complete Future). But in this case, the last thing that was happening was a call to do_request_json_for - which returns a Future - so there was no need for an extra call to Future->done, and your final then block was a no-op.

@clokep clokep merged commit 9113b56 into develop Mar 4, 2020
@clokep clokep deleted the clokep/alias-delete-pl branch March 4, 2020 16:30
anoadragon453 added a commit that referenced this pull request Mar 24, 2020
…ease-v1.12.x

* origin/release-v1.12.0:
  Missing bits
  Use r0 instead of 'unstable' for joined_members|rooms
  Add test to optionally keep other sessions when modifying a user's password. (#832)
  Remove tests that call query_auth endpoint. (#827)
  Add tests for validating alt_aliases of a canonical alias event. (#824)
  Update the haproxy config to route fallback auth and register to the same worker. (#828)
  Add tests for a user with sufficient power level removing an alias. (#818)
  Add a test for an event with an invalid signature (#821)
  Fix race in self signing key tests. (#822)
  Use retry_until_success instead of repeat_until_true.
  Attempt to deflake a test.
  Fix frontend proxy worker binding to wrong port. (#823)
  Stop checking for aliases in the public room list.
  Attempt to deflake a sytest (#816)
  Fix formatting from review.
  Support running Dendrite in more than 1 sqlite db file
  Add tests for moving alt_aliases to an upgraded room.
  Support running dendrite in sqlite
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