Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Skip tests for non-supported isolation levels with Oracle #7756

Merged
merged 1 commit into from Sep 25, 2012

Conversation

Projects
None yet
3 participants
Contributor

yahonda commented Sep 25, 2012

Since Oracle enhanced adapter supports specifying isolation levels with rsim/oracle-enhanced@de100e9

Oracle database itself does not support :read_uncommitted nor :repeatable_read. Tess for these isolation levels should be skipped.

@rafaelfranca rafaelfranca added a commit that referenced this pull request Sep 25, 2012

@rafaelfranca rafaelfranca Merge pull request #7756 from yahonda/isolation_level_oracle
Skip tests for non-supported isolation levels with Oracle
01cef4f

@rafaelfranca rafaelfranca merged commit 01cef4f into rails:master Sep 25, 2012

Member

jonleighton commented Sep 25, 2012

@yahonda it would be better if we wrote this as skip "..." unless conn.transaction_isolation_levels.include?(:read_uncommitted). That way we don't have to have specific skips for certain adapters. Thanks.

Contributor

yahonda commented Sep 25, 2012

Thanks for the update. Will update my commit.

Owner

rafaelfranca commented Sep 25, 2012

But to this work I think you will have to change the OracleAdpter to return the supported values

Contributor

yahonda commented Sep 25, 2012

@rafaelfranca This method should work I think, doesn't it?

rsim/oracle-enhanced@de100e9#L0R798

Owner

rafaelfranca commented Sep 25, 2012

Yes. It works.

Contributor

yahonda commented Sep 25, 2012

I made a commit yahonda/rails@0355523
I've squashed two commits, one with specific adapter name and another one does not depend on specific adapter name into one.

My previous commit has been merged to master branch, Do I need to open a pull request to revert 01cef4f first?

Owner

rafaelfranca commented Sep 25, 2012

You will need to open a pull request changing the code, but no need to revert the commit

Contributor

yahonda commented Sep 25, 2012

Thanks. Opened #7757.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment