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

Paginator functional tests #1342

Closed

Conversation

billschaller
Copy link
Member

PaginationTest has been expanded and now covers ordering and limiting rather thoroughly.

4 tests fail on PostgreSQL and MySQL, 14 tests fail on SQL Server, and I didn't try any other DBMS.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3629

We use Jira to track the state of pull requests and the versions they got
included in.

@billschaller
Copy link
Member Author

@Ocramius @stof @deeky666 aww yiss

@billschaller
Copy link
Member Author

With the changes from #1337, tests on pgsql and mysql pass, and there are 5 failures, and 7 errors on SQL Server.

doctrine/dbal#818 resolves all the SQL server failures, and all but 3 of the errors.

@billschaller
Copy link
Member Author

A fix to #1337 makes all tests pass on pgsql, mysql, and sql server. Anyone up for testing other platforms?

@Ocramius
Copy link
Member

wow

@Ocramius
Copy link
Member

Bonus for opening PR #1337:

p0wnz0r


use Doctrine\ORM\ORMException;

class PaginationException extends ORMException
Copy link
Member

Choose a reason for hiding this comment

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

Should probably extend RuntimeException in order to avoid BC breaks.

@billschaller
Copy link
Member Author

@Ocramius fixed all the things

}

/**
* @expectedException
Copy link
Member

Choose a reason for hiding this comment

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

this should be removed give you set it in the test itself

@stof
Copy link
Member

stof commented Mar 20, 2015

@Ocramius @deeky666 I'm in favor of merging this PR as soon as my few comments are fixed so that other PRs fixing the pagination logic can run on top of these new tests and prove they are fixing things

@billschaller
Copy link
Member Author

@stof fixed per your comments

@Ocramius Ocramius self-assigned this Mar 22, 2015
Ocramius added a commit to Ocramius/doctrine2 that referenced this pull request Mar 23, 2015
Ocramius added a commit that referenced this pull request Mar 24, 2015
…sues' into hotfix/#1342-paginator-functional-test-integration
Ocramius added a commit that referenced this pull request Mar 24, 2015
…sues' into hotfix/#1342-paginator-functional-test-integration
@Ocramius
Copy link
Member

Merged together with #1337

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.

None yet

4 participants