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

Bring SqlByPassTest to light and fix broken tests #6136

Merged
merged 1 commit into from May 16, 2012

Conversation

mhfs
Copy link
Contributor

@mhfs mhfs commented May 3, 2012

Test file activerecord/test/cases/session_store/sql_bypass.rb wasn't sufixed by _test.rb and so wasn't caught by the test suite.

Also, SqlBypass implemented a new_record? interface while the tests where expecting it to offer persisted?. Since this was part of a commit that was then reverted I wasn't sure which one was preferred. I went with persisted?.

And last but not least, forced session_id to be passed as a string to the sql queries since postgresql was complaining about type mismatch (string column vs integer value on where clause).

Cheers

@rafaelfranca
Copy link
Member

cc @jeremy

@mhfs
Copy link
Contributor Author

mhfs commented May 4, 2012

Maybe @tenderlove can take a look. I believe he's the one who created these tests.

@mhfs
Copy link
Contributor Author

mhfs commented May 15, 2012

/ping @rafaelfranca @tenderlove @jeremy :)

@rafaelfranca
Copy link
Member

I'm not sure a about the new_record? since it is public API I would create a persisted? method that returns !@new_record.

@mhfs
Copy link
Contributor Author

mhfs commented May 16, 2012

@rafaelfranca makes sense. Just brought new_record? back and added persisted? as an alternative to keep it symmetric with AR's persistence. What do you think now?

@rafaelfranca
Copy link
Member

could you squash the commits?

…ted?` for SqlBypass as expected by tests and convert session_id to string before using on queries to get correct quotes on postgresql (avoid casting error).
@mhfs
Copy link
Contributor Author

mhfs commented May 16, 2012

@rafaelfranca sure. there you go.

tenderlove added a commit that referenced this pull request May 16, 2012
Bring SqlByPassTest to light and fix broken tests
@tenderlove tenderlove merged commit 2a67a8c into rails:master May 16, 2012
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

3 participants