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

[Tests-Only] Refactor acceptance tests #394

Merged
merged 1 commit into from
Mar 2, 2020

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Mar 2, 2020

Description

Before adding new test scenarios for PR #384 , refactor the existing acceptance tests:

  • improve scenario descriptions
  • split Given and When steps
  • add better error messages to Assert checks

How Has This Been Tested?

Local run of acceptance tests

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@@ -169,19 +169,6 @@ Feature: Guests
Then the OCS status code should be "404"
And the HTTP status code should be "200"

Scenario: Check that skeleton is properly set
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This scenario was not related to guests at all

/**
* @return string|null
*/
public function getGuestGroupName() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was not used.

* @return void
* @throws Exception
*/
public function guestUserHasBeenCreatedWithEmailAndPassword($user, $email, $password) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was not used.

@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #394 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #394   +/-   ##
=========================================
  Coverage     17.85%   17.85%           
  Complexity       96       96           
=========================================
  Files            11       11           
  Lines           532      532           
=========================================
  Hits             95       95           
  Misses          437      437           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d731e1d...3f9f908. Read the comment docs.

Copy link
Contributor

@jasson99 jasson99 left a comment

Choose a reason for hiding this comment

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

LGTM

@phil-davis phil-davis requested a review from individual-it March 2, 2020 06:06
@individual-it individual-it merged commit ceb68cb into master Mar 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the refactor-acceptance-tests branch March 2, 2020 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants