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 missing PHPUnit tests to Travis CI #1540

Merged
merged 18 commits into from Aug 25, 2019

Conversation

@dregad
Copy link
Member

commented Aug 9, 2019

Travis CI builds are not executing all of the defined PHPUnit test suites : IssueHistoryTest and REST tests are not executed.

This PR makes changes to both the PHPUnit scripts and the Travis configuration, to ensure all tests are run and we don't have skipped tests either.

@dregad

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2019

This includes @vboctor's fix in PR #1538.

@dregad

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2019

@vboctor I need some help with this one.

One of the REST tests is still failing (RestIssueAddTest::testCreateIssueWithTags) and I do not know how to fix that. I do not understand why the issue body is returned as NULL, when as far as I can tell we're executing the same command as in other test cases (e.g. testCreateIssueWithVersionObjectId), and the body is set correctly in that case.

Also, I had to comment out some of the assertions in testCreateIssueWithTagIdNotFound and testCreateIssueNoSummary, because the REST API returns a 404 as expected, but I have no way of retrieving the created issue so I have nothing to compare against.

@dregad

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

@vboctor awating your feedback on this

@vboctor

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

@dregad This is probably caused by a bug in the IssueAddCommand. The command expects the array of tags to be an array of arrays, each one containing an id and a name. The sample expected usage can be found in bug_report.php. The fix is to detect when ids are not provided and lookup the tag id from the provided name.

@dregad

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

@vboctor that was it indeed ! Thanks for pointing me in the right direction.

The code in IssueAddCommand is accessing $t_tag['id'] without first checking that the key exists, so when a tag is provided only by name, PHP throws a NOTICE causing the API to return an invalid JSON response (there is an HTML error message appended to it).

I logged https://mantisbt.org/bugs/view.php?id=25997 to track the bug.

You have not responded to the other issue I am facing

I had to comment out some of the assertions in testCreateIssueWithTagIdNotFound and testCreateIssueNoSummary, because the REST API returns a 404 as expected, but I have no way of retrieving the created issue so I have nothing to compare against.

IMO, this is a design flaw of the API's current implementation, which first creates the issue, then creates/attaches tags to it as a separate, second step.

@dregad dregad referenced this pull request Aug 14, 2019
@dregad dregad force-pushed the dregad:travis-fixes branch from a873844 to d9e798d Aug 14, 2019
@dregad

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

Would it be the correct approach, to execute in IssueAddCommand::validate(), the same checks on tags to attach, as in mci_tag_set_for_issue() ?

If yes, in the interest of avoiding duplication, this code should be moved to a function; any advice on where to put it ?

@dregad dregad force-pushed the dregad:travis-fixes branch 3 times, most recently from 9435c1a to aee455a Aug 19, 2019
@dregad dregad force-pushed the dregad:travis-fixes branch from aee455a to 2ec8e37 Aug 25, 2019
@dregad dregad changed the title WIP: Add missing PHPUnit tests to Travis CI Add missing PHPUnit tests to Travis CI Aug 25, 2019
@dregad

This comment has been minimized.

Copy link
Member Author

commented Aug 25, 2019

This is now ready to merge (after #1542).

dregad and others added 14 commits May 2, 2019
When creating an issue with non-default status/resolution, check that a single entry is created.

Fixes #25962
With previous configuration, IssueUpdateTest::testUpdateWithRareFields
was not executed.
Dynamically generate an API token for the administrator user to execute
the REST API test suite.
Code copied from MantisCoreBase suite.
TODO: check if code duplication can be avoided
Update before_script to create 3 version records.

This is used by REST tests.
Prior to this, the tests were written to use harcoded version numbers
and ids taken from mantisbt.org database.

Now we retrieve versions from the database; there must be at least 3
defined versions for the test project.
The assertion is now conditional, based on how $g_allow_no_category is
set. This option defaults to OFF, but Travis builds set it to ON to
avoid skipping tests in SOAP suite.
Prior to this, date order was set to a hardcoded timestamp. This caused
testAddVersion to fail if there were any existing Version records in the
database having a more recent date order more recent.

To fix the problem, the DATE_ORDER constant has been replaced by a
$date_order class property, set to current date/time when the test suite
starts.
The following tests have been removed, as they are now covered by the
new testCreateIssueWithTagInvalid:
- testCreateIssueWithTagNameNotFound
- testCreateIssueWithTagIdNotFound
dregad added 4 commits Aug 19, 2019
In addition to string (json), it can also be array or object.
The tag should be created together with the issue.
The tag should be attached to the issue.
@dregad dregad force-pushed the dregad:travis-fixes branch from 2ec8e37 to 399f65c Aug 25, 2019
dregad added a commit that referenced this pull request Aug 25, 2019
This also fixes and improves RestIssueAddTest, which were not correct
but nobody noticed since they were never executed.

Merge PR #1540
@dregad dregad merged commit 399f65c into mantisbt:master Aug 25, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dregad dregad deleted the dregad:travis-fixes branch Aug 25, 2019
@dregad dregad restored the dregad:travis-fixes branch Aug 25, 2019
@dregad dregad deleted the dregad:travis-fixes branch Aug 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.