Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

More tests #729

Merged
merged 15 commits into from
Oct 17, 2023
Merged

More tests #729

merged 15 commits into from
Oct 17, 2023

Conversation

thesuzerain
Copy link
Contributor

@thesuzerain thesuzerain commented Oct 13, 2023

Fixes MOD-556

Copy link
Member

@Geometrically Geometrically left a comment

Choose a reason for hiding this comment

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

For my comment on the permissions test, I'm thinking we apply that to all of the permissions tests (move them to the respective object file which is being modified!)

pub struct OrganizationPermissions: u64 {
const EDIT_DETAILS = 1 << 0;
const EDIT_BODY = 1 << 1;
const UNUSED = 1 << 1; // a currently unused permission, formerly EDIT_BODY
Copy link
Member

Choose a reason for hiding this comment

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

since this is unused why not just remove the flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was trying to avod breakage in staging- but probably doesnt matter

pub struct DummyData {
// Alpha project:
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better for this to use nested structs instead of one top-level one

tests/common/permissions.rs Show resolved Hide resolved
Comment on lines 62 to 65
if member.accepted {
return Some(member.permissions);
}
return Some(member.permissions);
Copy link
Contributor

Choose a reason for hiding this comment

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

As written, this change has no effect on the behavior of the code - it just always returns Some(member.permissions). Did you maybe intend to only return that when the member has accepted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops- this is an artifact from that test I commented out.

@OmegaJak
Copy link
Contributor

Looking forward to using the new templated test DB!

@thesuzerain thesuzerain marked this pull request as ready for review October 17, 2023 00:26
@thesuzerain thesuzerain merged commit 9d0e762 into master Oct 17, 2023
6 checks passed
@thesuzerain thesuzerain deleted the more-tests branch October 17, 2023 07:53
thesuzerain added a commit that referenced this pull request Dec 5, 2023
* permissions tests

* finished permissions; organization tests

* clippy, fmt

* post-merge fixes

* teams changes

* refactored to use new api

* fmt, clippy

* sqlx prepare

* revs

* revs

* re-tested

* re-added name

* reverted to matrix
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants