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

Rename integration API tests to acceptance #30493

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Feb 15, 2018

Description

Rename the tests/integration folder to tests/acceptance and related search/replace.

Related Issue

owncloud/QA#517

Motivation and Context

The API tests that are currently referred to as "integration" tests are really part of the acceptance tests.
Rename the folder to acceptance. Then as a later step we can sort out merging the UI acceptance tests into here also (which already share bits of code in TestHelpers and a similar Behat-Gherkin framework)

How Has This Been Tested?

Run some of the API tests locally.
CI will know if I missed anything.

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)
  • Test refactoring

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.

@phil-davis
Copy link
Contributor Author

phil-davis commented Feb 15, 2018

This looks quite easy to do. Paths of various PSR loader things are all relative, so it should "just work".

"integration" tests in apps will need to be modified similarly so they find the core test code in acceptance - that should be easy to do also.

If people like the look of this, I can get the associated bits ready easily.

Copy link
Contributor

@SergioBertolinSG SergioBertolinSG left a comment

Choose a reason for hiding this comment

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

I don't like this name change.

@codecov
Copy link

codecov bot commented Feb 15, 2018

Codecov Report

Merging #30493 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #30493   +/-   ##
=========================================
  Coverage      61.9%    61.9%           
  Complexity    19065    19065           
=========================================
  Files          1091     1091           
  Lines         61488    61488           
=========================================
  Hits          38063    38063           
  Misses        23425    23425

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 de54da5...61a44bd. Read the comment docs.

@patrickjahns
Copy link
Contributor

patrickjahns commented Feb 15, 2018

@SergioBertolinSG

Please state a reason - a "I don't like" is not a valid decline.

Generally speaking - we've talked before - the current "integration" tests are end-to-end tests of the API.

They do not focus on testing the integration of various components (units) with each other

@phil-davis phil-davis force-pushed the rename-integration-to-acceptance branch from 6d619a5 to 6958bcc Compare February 15, 2018 17:15
@SergioBertolinSG
Copy link
Contributor

Integration tests are checking that some of the parts of a system are working well together.
Acceptance tests are checking that some of the final user actions are working correctly.

I am not considering the backend as the final system that the user is going to use. It requires a frontend or a client (desktop or mobile).

I guess the confusion comes with the two kinds of integration testing, the ones close to the unit tests, using different classes and these following the behaviour of the api/webdav endpoints.

Having said this, if you all agree with the change, please go ahead.

@phil-davis
Copy link
Contributor Author

In the case of the "server", the core server repo "product" is:

  1. the behaviour of the API, for which the "users" are the clients that use the API (iOS, Android, Windows, Mac, webUI)
  2. the behaviour of the occ command, for which the "users" are admins that have command line access
  3. the behaviour of the webUI, for which the "users" are what we traditionally think of when we say "users" - actual "normal" people out in the world
    • (3) is actually the odd one here, the "server" has a "client app" bundled/delivered with it, the browser-based webUI.

Testing the external "black-box" behaviour of any of these can be termed "acceptance" testing.

In our case, we are using the same Behat-Gherkin framework do drive these tests. So it is possible to (one day) put them all together in a tests/acceptance folder. (But actually we could choose to keep them in 3 separate folders)

@phil-davis phil-davis force-pushed the rename-integration-to-acceptance branch 5 times, most recently from f9ea30e to 975fb2b Compare February 27, 2018 02:40
@patrickjahns
Copy link
Contributor

In our case, we are using the same Behat-Gherkin framework do drive these tests. So it is possible to (one day) put them all together in a tests/acceptance folder. (But actually we could choose to keep them in 3 separate folders)

I don't mind if we keep the specification/feature files in separate test folders, but I think it makes sense to build a common library / framework for how to blackbox test owncloud (wether its via api/occ/ui)

@phil-davis phil-davis force-pushed the rename-integration-to-acceptance branch from 975fb2b to 61a44bd Compare February 27, 2018 11:16
@phil-davis
Copy link
Contributor Author

Backport stable10 #30499

@phil-davis
Copy link
Contributor Author

phil-davis commented Feb 28, 2018

Related changes required in app repos so they use core integration test code from the right place...

@phil-davis
Copy link
Contributor Author

All related PRs have been merged. This is finished.

@phil-davis
Copy link
Contributor Author

Documentation issue owncloud-archive/documentation#3854

@lock
Copy link

lock bot commented Jul 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants