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

Travis CI Integration #319

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Travis CI Integration #319

wants to merge 6 commits into from

Conversation

gohanman
Copy link
Contributor

I noticed #152 in the issues list. This seems like a good one to check off before getting too deep into a refactor like #317. It's also strictly new files so it shouldn't conflict with the current PHPDoc effort. I did notice PR #161 on the same subject but that doesn't seem to have any tests beyond just opening a connection to a database.


Adds Travis-CI integration and PHPUnit tests.

This includes MySQL (mysqli), Postgres (postgres9), and SQLite (sqlite3) plus the PDO variants of each. Tests can also be run locally by creating a tests/credentials.json file formatted:

{
  "postgres": { "user": "username", "password": "password" },
  "mysql": { "user": "username", "password": "password" }
}

Tests that have been ported from existing scripts are test-active-record.php, test-datadict.php, and test-xmlschema.php. I also added a number of new tests calling various methods of ADOConnection.

Add Travis-CI integration and PHPUnit tests.

This includes MySQL (mysqli), Postgres (postgres9), and SQLite (sqlite3) plus the PDO variants of each. Tests can also be run locally by creating a `tests/credentials.json` file formatted:
```json
{
  "postgres": { "user": "username", "password": "password" },
  "mysql": { "user": "username", "password": "password" }
}
```

Tests that have been ported from existing scripts are `test-active-record.php`, `test-datadict.php`, and `test-xmlschema.php`. I also added a number of new tests calling various methods of `ADOConnection`.
@peterdd
Copy link
Contributor

peterdd commented Mar 22, 2017

So you want write the same tests for each database again? So each such test file must be maintained and synced? I would prefer a solution without redundancy.

I wish someone could pickup my rough idea #161 , review, and fill it with real tests.
I by myself are a little bit stuck with my progress on phpunit learning, so bringing it to life on ADOdb would be great.

@gohanman
Copy link
Contributor Author

So you want write the same tests for each database again? So each such test file must be maintained and synced?

Not the exact same tests. The actual capabilities of the different databases (as well as the PDO / non-PDO variants) are different enough that it seems like the better option. To have a single set of tests running on all of them, the test set would have to be really limited in what it covers or be riddled with conditionals for tests that have different inputs and/or outputs with a given driver or tests that should simply be skipped for a given driver. I think the repetitiveness is worthwhile to have logically straightforward test methods that simply call a serious of functions in order. Less complicated test methods are less likely to contain logic bugs and thus less likely to require maintenance anyway. Ideally the test should only need updates if the behavior of tested functionality is intentionally changed.

@peterdd
Copy link
Contributor

peterdd commented Mar 22, 2017

The purpose of ADOdb:

ADOdb is a PHP database class library that provides powerful abstractions for performing queries and managing databases. ADOdb also hides the differences between DB engines so you can easily switch them without changing your code.

Having single sourced test cases for the ADOdb / SQL core features most databases provide is important I think.

For databases who have extended capabilities beyond SQL standards and where ADOdb supports it, extra test cases can be written additional, no problem.

For databases that only support a subset of SQL or the ADOdb hasn't implemented yet, that tests can be skipped. For example if a database driver doesn't support foreign keys, that tests will get skipped. An example would be MySQL MyISAM engine would skip the foreign key test cases.

testresults

@gohanman
Copy link
Contributor Author

I don't disagree in principle and have no objection to having a subset of tests that are expected to run on all databases with identical code. I just think those are going to wind up being the minority.

Let's compare postgres9 and pdo_pgsql. Say my test wants to create a table. If I call NewDataDictionary on the PDO connection I get an error. So right out the gate I need some driver-specific code in the tests. Once I create the table I can call MetaColumns on either driver and get information about the table... but the two drivers return different ADOFieldObjects even if the underlying table is identical.

Next I might want to insert a record. Calling BeginTrans works with postgres9 but throws an exception with pdo_pgsql. If I leave things in autocommit mode I can insert a record with identical code for each driver. But if I want the ID of the record I just created Insert_ID only works correctly with postgres9.


Some of these things, arguably, are bugs that could be fixed. ADOdb could do a better job hiding some of these differences. But I'm aiming to keep this PR strictly tests without changing any existing functionality.

Adds AllTest to contain tests that will run on all available drivers.
@gohanman
Copy link
Contributor Author

b7f5de6 adds a place to put test code that can run correctly against all six drivers.

@mnewnham
Copy link
Contributor

mnewnham commented Aug 5, 2018

I recognize that there has been some delay in responding to this, and I don't know if you are still interested in this project, but I have just cloned your code to have at look at it. I have a couple of thoughts regarding the code so far:

  1. Would you consider separating your code out from the existing tests, perhaps moving them to a /travis directory.
  2. Could you perhaps write a short explanatory readme for your tests: what they are for, how they work, requirements, setup and why someone might want to use them in regards to ADOdb. feel free to link to the appropriate explanatory pages. I consider myself an ideal candidate for such an explanation, I'm an IT professional with 40 years programming experience, PHP for 15 years, I currently manage a massive PHP project with 30.000 programs and 2 million lines of code, I've contributed to ADOdb for 15 years as well, but I've never used travis and I've no real idea what the point is.....

@gohanman
Copy link
Contributor Author

gohanman commented Aug 6, 2018

I'll look at rearranging code and adding comments but wanted to respond to this part:

I've never used travis and I've no real idea what the point is.....

In a nutshell, automation. On a pull request and/or commit to master, Travis-CI will launch a Docker instance with a well-defined environment, clone to code, run the tests, and report success or failure. I think the automation piece is generally useful since it becomes impossible to forget to run the tests.

With ADOdb specifically I don't think it makes sense to mock out the database. Such a database-centric library should get tested against real databases and Travis-CI provides a way to test against multiple databases without making the developer install & configure them all locally.

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.

None yet

3 participants