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 more tests for Permissions class #967

Merged
merged 7 commits into from Mar 2, 2018

Conversation

Projects
None yet
2 participants
@Chris-Johnston
Copy link
Contributor

commented Feb 28, 2018

Added more tests to the Permissions class that tries to cover more of the methods than were already being tested. These are similar to the existing tests for GuildPermissions and ChannelPermissions but work more closely with Permissions by converting them to ulong raw values first.

Added tests for OverwritePermissions that checks that values are set correctly and that flags can be toggled correctly, including when both allow and deny are set for the same bit.

Tried to make tests fail with issue described in #965 but I was unable to find a case that broke the tests. Would appreciate input and any suggestions on making a test case for that. (CC: @FiniteReality )

My discord handle is @ChrisJ#8703 should you need to ping/DM me.

@@ -24,4 +27,7 @@
<PackageReference Include="xunit.runner.visualstudio" Version="2.2.0" />
<PackageReference Include="xunit.runner.reporters" Version="2.2.0" />
</ItemGroup>
<ItemGroup>
<Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />

This comment has been minimized.

Copy link
@foxbot

foxbot Feb 28, 2018

Member

what's this?

This comment has been minimized.

Copy link
@Chris-Johnston

Chris-Johnston Mar 1, 2018

Author Contributor

I'm not sure, I don't recall adding anything explicitly. I'll remove it from the file.

Chris-Johnston added some commits Mar 1, 2018

Separate out GuildPermissions and ChannelPermissions tests from main …
…partial Tests class because they do not need to have access to the main test fixture
@Chris-Johnston

This comment has been minimized.

Copy link
Contributor Author

commented Mar 1, 2018

I may be making a bad habit out of doing more than one thing in a PR.
The GuildPermissions and ChannelPermissions classes didn't need to be part of the Test class, and would fail if the test fixture wasn't supplied. I moved them into their own classes.

@foxbot foxbot merged commit 63e6704 into discord-net:dev Mar 2, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@Chris-Johnston Chris-Johnston deleted the Chris-Johnston:add-permissions-tests-966 branch Mar 2, 2018

FiniteReality added a commit to FiniteReality/Discord.Net that referenced this pull request May 5, 2018

Add more tests for Permissions class (discord-net#967)
* Add tests for more Permissions code coverage

* Add guild tests

* Add more in-depth covering of permissions methods

* Add tests for OverwritePermissions

* Remove unknown ItemGroup tag from csproj

* Add missing Fact attributes, separate class so that it is not dependant on main test fixture

* Separate out GuildPermissions and ChannelPermissions tests from main partial Tests class because they do not need to have access to the main test fixture
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.