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

[API] 2020-05-25 Add api testsuite #6663

Closed

Conversation

spell00
Copy link
Contributor

@spell00 spell00 commented Jun 3, 2020

Creation of a test suite for the API

@spell00 spell00 closed this Jun 3, 2020
@spell00 spell00 reopened this Jun 3, 2020
@spell00 spell00 marked this pull request as draft June 3, 2020 03:32
@johnsaigle johnsaigle added the Needs formatting PR requires formatting fixes to meet our coding standards (PHPCS, phan, etc.) label Jun 3, 2020
@johnsaigle
Copy link
Contributor

This is failing Travis because of code formatting issues. If you run make checkstatic in the LORIS root directory you'll be able to see locally what the issues are.

Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

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

Here are some feedbacks.

@johnsaigle @kongtiaowang All comments are welcome but this is a draft PR I requested. It is not ready yet :)



namespace LORIS\api\Test;
ini_set('memory_limit', '1024M');
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed.
The testsuite will fail if something is wrong with memory limit and we will look at the issue globaly.

*
* @category API
* @package Tests
* @subpackage Login
Copy link
Contributor

Choose a reason for hiding this comment

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

Change Login for integration maybe

modules/api/test/ApiTest.php Outdated Show resolved Hide resolved
modules/api/test/ApiTest.php Show resolved Hide resolved

require './vendor/autoload.php';

use PHPUnit\Framework\TestCase;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed if you require LorisIntegrationTest.class.inc

modules/api/test/ApiTest.php Outdated Show resolved Hide resolved
modules/api/test/ApiTest.php Outdated Show resolved Hide resolved
modules/api/test/ApiTest.php Outdated Show resolved Hide resolved
{
$this->_base_uri = 'https://test-loris-dev.loris.ca/api/v0.0.3/';
$this->_client = new \GuzzleHttp\Client(['base_uri' => $this->_base_uri]);
$response = $this->_client->request('POST', LORIS_URL, ['json' => ['username' => LORIS_USERNAME, 'password' => LORIS_PASSWORD]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is LORIS_URL?

Copy link
Contributor

Choose a reason for hiding this comment

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

LORIS_USERNAME should be UnitTester and LORIS_PASSWORD should be $this->validPassword
Right @kongtiaowang ?

modules/api/test/ApiTest.php Outdated Show resolved Hide resolved
@spell00 spell00 changed the title [2020 05 25 add api testsuite [API] 2020-05-25 Add api testsuite Jun 4, 2020
@spell00 spell00 closed this Jun 4, 2020
@spell00 spell00 force-pushed the 2020-05-25-AddApiTestsuite branch from 3f0a3d1 to 54160f3 Compare June 4, 2020 22:57
@spell00
Copy link
Contributor Author

spell00 commented Jun 5, 2020

This branch is to be deleted. The new branch is #6671.

@spell00 spell00 deleted the 2020-05-25-AddApiTestsuite branch June 5, 2020 02:51
@christinerogers
Copy link
Contributor

@spell00 important to mention -- Are @xlecours 's outstanding comments (above) covered in #6671 already, or will they be in your next commit ?

@spell00
Copy link
Contributor Author

spell00 commented Jun 5, 2020

Yes, all comments, including @xlecours 's comments above, are covered in #6671

@christinerogers christinerogers removed the Needs formatting PR requires formatting fixes to meet our coding standards (PHPCS, phan, etc.) label Jun 8, 2020
@johnsaigle
Copy link
Contributor

Screen Shot 2020-06-08 at 11 08 52

@spell00 I think something may have gone wrong in the commit. 2000+ files seems like a lot

@aces aces deleted a comment from kongtiaowang Jun 8, 2020
@christinerogers
Copy link
Contributor

fyi @johnsaigle you're seeing the commit necessary to rollback an unfortunate commit

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.

5 participants