# Code Review: rails2

**URL:** <https://www.ruby-forum.com/t/code-review-rails2/182042>\
**Category:** IronRuby\
**Created:** [January 21, 2010, 11:02pm UTC](https://www.ruby-forum.com/t/code-review-rails2/182042 "2010-01-21T23:02:53Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 21, 2010, 11:02pm UTC](https://www.ruby-forum.com/t/code-review-rails2/182042/1 "2010-01-21T23:02:53Z")

</div>

tfpt review “/shelveset:rails2;REDMOND\sborde”  
Comment :  
Added active\_resource and action\_mailer tests to irtests.rb  
Added a category (disable\_unstable\_tests) of unstable tests to utr.rb  
so that we can track non-deterministic failures. Also a category  
(exclude\_critical\_files) for critical test files that cause an exception  
when loaded (even if no tests are run from the file).  
Disabled a TestMultiTask test in rake\_tests. MultiTaskTest uses  
monitor.rb which uses Thread.critical= in a way that is not friendly  
with native threads. Ideally, we will implement our own version of  
monitor.rb that does not uses Thread.critical=  
Also, I think I left out active\_support\_tests.rb out of my previous  
update to 2.3.5. So it might be broken in irtests. This change should  
fix it.

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 21, 2010, 11:19pm UTC](https://www.ruby-forum.com/t/code-review-rails2/182042/2 "2010-01-21T23:19:52Z")

</div>

- irtests.rb: Reminder for me: why is active record commented out?
- utr.rb: Disable\_unstable\_tests doesn’t appear to get called, is that  
intentional?
- active\_record\_tests.rb, line 82: I would get rid of the empty method,  
it’s already defined

Otherwise looks good.

JD

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 22, 2010, 1:49am UTC](https://www.ruby-forum.com/t/code-review-rails2/182042/3 "2010-01-22T01:49:09Z")

</div>

To enable active record tests, we need to check in the  
ironruby-sqlserver gem (not hard), and implement ensure\_test\_databases  
(a bit more work to figure out).

disable\_unstable\_tests should be called. Good catch.

Removed line 82 of active\_record\_tests.rb

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 22, 2010, 1:55am UTC](https://www.ruby-forum.com/t/code-review-rails2/182042/4 "2010-01-22T01:55:00Z")

</div>

Sounds good