Forum: Rails-core (closed, excessive spam) Adding to the tables defined for AR's unit tests

81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-10 22:36
(Received via mailing list)
Hi,

I noticed that in the association_preload there are a number of FIXMEs
that all boil down to the fact that I ignored table/column name
quoting when i first wrote it up.

Ideally to test this I'd need a bunch of tables with interesting table
& column names. What's the policy on adding to the tables/models using
in tests? Just do it ?  Also I noticed there's a whole bunch of files
in schema for different databases: does one need to add the
declaration for such tables to all of these files? I don't have access
to all of those dbs so it would be hard for me to be sure I hadn't
messed things up.

Fred
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-11 01:13
(Received via mailing list)
>  Ideally to test this I'd need a bunch of tables with interesting table
>  & column names. What's the policy on adding to the tables/models using
>  in tests? Just do it ?  Also I noticed there's a whole bunch of files
>  in schema for different databases: does one need to add the
>  declaration for such tables to all of these files? I don't have access
>  to all of those dbs so it would be hard for me to be sure I hadn't
>  messed things up.

It would be *really* nice if we could tidy up all those active record
tests to use schema.rb, thereby removing most of the hoops you have to
jump through to add a table to the test schema.  Unfortunately this is
probably a reasonably large undertaking, so if your time is limited
you're probably best off just getting it working on your own
databases, making an educated guess for the others, and asking people
for help testing.

I personally have an EC2 image I use to test rails on all the other
open source databases.

--
Cheers

Koz
3d3c200d0a9fbb17a3bcaf1c3e8c2eeb?d=identicon&s=25 Jordi Bunster (Guest)
on 2008-04-11 02:48
(Received via mailing list)
Attachment: smime.p7s (4 KB)
On Apr 10, 2008, at 7:12 PM, Michael Koziarski wrote:

> I personally have an EC2 image I use to test rails on all the other
> open source databases.

That sounds very useful. Is that an AMI you can make public?
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-11 17:25
(Received via mailing list)
On 11 Apr 2008, at 00:12, Michael Koziarski wrote:

>> messed things up.
>
> It would be *really* nice if we could tidy up all those active record
> tests to use schema.rb, thereby removing most of the hoops you have to
> jump through to add a table to the test schema.  Unfortunately this is
> probably a reasonably large undertaking, so if your time is limited
> you're probably best off just getting it working on your own
> databases, making an educated guess for the others, and asking people
> for help testing.
>

I'm sure I'm showing my ignorance here, but I don't quite get what
needs to be done.

Part of schema.rb is bracketed with if adapter == 'MYSQL', with a
comment about things being transitioned from fixtures/db_definitions
(which doesn't seem to exist). Those tables that are in the "if mysql"
bit are not in sqlite.sql, postgresql.sql etc..., but those bits that
aren't are not in the database specific .sql files, so the other
databases are obviously using schema.rb partially.
Why is it not a case of just removing the "if mysql" bits ? I'm happy
to spend some time looking at this but I don't think I get what's to
be done.

Fred
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-12 00:26
(Received via mailing list)
>  Why is it not a case of just removing the "if mysql" bits ? I'm happy
>  to spend some time looking at this but I don't think I get what's to
>  be done.

That's about all there is to it,  but after you remove it you have to
run the tests, and catch any differences (implicit or otherwise)
between the schema.rb definition and the schema.sql.

So it's not hard, just time consuming.


--
Cheers

Koz
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-12 00:28
(Received via mailing list)
>  That sounds very useful. Is that an AMI you can make public?

I'll have to rebuild it to use the new git repository and make sure I
don't have my AWS keys in the log files somewhere, but it's certainly
something I'm considering doing.  Unfortunately I'm not sure when I'll
get the requisite free time.   If someone feels hugely motivated and
grabs me on IRC I'm happy to share the tricks / tips that I learned
along the way


--
Cheers

Koz
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-14 14:29
(Received via mailing list)
On 11 Apr 2008, at 23:25, Michael Koziarski wrote:

>
>> Why is it not a case of just removing the "if mysql" bits ? I'm happy
>> to spend some time looking at this but I don't think I get what's to
>> be done.
>
> That's about all there is to it,  but after you remove it you have to
> run the tests, and catch any differences (implicit or otherwise)
> between the schema.rb definition and the schema.sql.

I've had a bash at this, and I've got things in a state where things
pass in mysql (obviously) and sqlite3 and postgresql. I haven't the
faintest clue what will happen with other databases, mostly because I
don't have them installed and know even less about those than the rest.

There are some issues I've had along the way though. The schemas
aren't quite identical to what they were before, for example the sql
files for postgres defined a lot of strings has having length 50 (not
for any particular reason as far as I can tell) whereas schema.rb
defines them as having length 255. Another one was People.first_name
is not null in schema.rb but allows null in postgres' sql file, which
caused a failing test (In that case i fixed up the test).

The main thing that is a bit fiddly is how to deal with database
differences. So for example postgres has a bunch of tables that are
used to test some postgres specific stuff. Right now at the bottom of
schema.rb there's a

case adapter_name
when 'Postgres'
   do some postgres stuff
...
end

and there's also one at the top of the file which can be used for
adapter specific setup. I haven't decided yet if maybe some of that
would be better factored out and put somewhere else (where ?)

Anyway, what i've got so far is over at
http://github.com/fcheung/rails/tree/master
, thoughts appreciated (if the git stuff is messed up it's probably
because I don't have the faintest foggiest clue what I'm doing - I've
not used git before, but loving it so far!)

Fred
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-16 01:58
(Received via mailing list)
On 14 Apr 2008, at 13:29, Frederick Cheung wrote:

>> run the tests, and catch any differences (implicit or otherwise)
>> between the schema.rb definition and the schema.sql.


I've put the current version of this patch on trac:
http://dev.rubyonrails.org/ticket/11598
Right now mysql, postgres and sqlite3 are happy. The databases I don't
have access to/am clueless about are probably unhappy.

Fred
2475563a3ba1da4018af64f964ab45b0?d=identicon&s=25 Chad Woolley (Guest)
on 2008-04-16 02:12
(Received via mailing list)
On Tue, Apr 15, 2008 at 4:52 PM, Frederick Cheung
<frederick.cheung@gmail.com> wrote:
>  I've put the current version of this patch on trac: 
http://dev.rubyonrails.org/ticket/11598
>  Right now mysql, postgres and sqlite3 are happy. The databases I don't
>  have access to/am clueless about are probably unhappy.

Which databases are these?  I mean, which databases is Rails actively
making an effort to keep the tests green against, and which ones are
known to not work anyway (or can accept breakages)?
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-16 02:20
(Received via mailing list)
On 16 Apr 2008, at 01:11, Chad Woolley wrote:

> known to not work anyway (or can accept breakages)?
Well the ones for which the adapters come with rails (mysql, postgres,
sqlite3, sqlite (does anyone actually use sqlite non-3? there's
currently failing tests there)) should certainly all be kept happy. I
personally don't have the faintest clue how heavily used/important the
others are.

Fred
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-17 09:25
(Received via mailing list)
>  Well the ones for which the adapters come with rails (mysql, postgres,
>  sqlite3, sqlite (does anyone actually use sqlite non-3? there's
>  currently failing tests there)) should certainly all be kept happy. I
>  personally don't have the faintest clue how heavily used/important the
>  others are.

The other databases should be kept green by the maintainers of their
adapters.   So sqlite, mysql and postgresql are the ones we're looking
after.   The sqlite2 test failures are strange, is there anyone out
there using it at present?  Would be good to fix this before 2.1 or at
least document the missing functionality.

--
Cheers

Koz
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-17 09:42
(Received via mailing list)
On Apr 17, 8:24 am, "Michael Koziarski" <mich...@koziarski.com> wrote:
> least document the missing functionality.
> =
I haven;t looked into it in any detail, but it's all stuff that is
currently broken in  trunk. I'll try and take a closer look soon.

Fred
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-17 11:28
(Received via mailing list)
On 17 Apr 2008, at 08:41, Frederick Cheung wrote:

>>> important the
> I haven;t looked into it in any detail, but it's all stuff that is
> currently broken in  trunk. I'll try and take a closer look soon.

In 2.0.2 quote_table_name was a no-op for the sqlite adapter (if not
overriden, which sqlite doesn't, it used to just return its argument,
but now by default it's just the same as quote_column_name). In
particular this messes with some of the eager loading stuff. for
example on sqlite 3,  running

SELECT DISTINCT "developers".id FROM "developers"  LEFT OUTER JOIN
"developers_projects" ON "developers_projects".developer_id =
"developers".id  LEFT OUTER JOIN "projects" ON "projects".id =
"developers_projects".project_id     WHERE (projects.id = 2)  LIMIT 1

returns the selected column as id whereas with sqlite2 it returns it
as "developers".id which leads to much confusion In the sqlite adapter
there's a key.sub(/^\w+\./, ''), but of course that doesn't catch it
now that the key is "developers.id" instead of developers.id. changing
the regex to /^"?\w+"?\./ fixes most of the failures, leaving me with

   1) Error:
test_should_count_manual_select_with_include(CalculationsTest):
ActiveRecord::StatementInvalid: SQLite::Exceptions::SQLException: near
"DISTINCT": syntax error: SELECT COUNT(*) AS
count_distinct_accounts_id FROM (SELECT DISTINCT DISTINCT accounts.id
FROM "accounts"  LEFT OUTER JOIN "companies" ON "companies".id =
"accounts".firm_id AND "companies"."type" = 'Firm'     )
     ./test/cases/../../lib/active_record/connection_adapters/
abstract_adapter.rb:151:in `log'
     ./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:132:in `execute_without_counting'
     ./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:353:in `catch_schema_changes'
     ./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:132:in `execute_without_counting'
     ./test/cases/helper.rb:38:in `execute'
     ./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:256:in `select'
     ./test/cases/../../lib/active_record/connection_adapters/abstract/
database_statements.rb:7:in `select_all_without_query_cache'
     ./test/cases/../../lib/active_record/connection_adapters/abstract/
query_cache.rb:55:in `select_all'
     ./test/cases/../../lib/active_record/connection_adapters/abstract/
database_statements.rb:13:in `select_one'
     ./test/cases/../../lib/active_record/connection_adapters/abstract/
database_statements.rb:19:in `select_value'
     ./test/cases/../../lib/active_record/calculations.rb:213:in
`execute_simple_calculation'
     ./test/cases/../../lib/active_record/calculations.rb:124:in
`calculate'
     ./test/cases/../../lib/active_record/calculations.rb:120:in
`calculate'
     ./test/cases/../../lib/active_record/calculations.rb:46:in `count'
     ./test/cases/calculations_test.rb:249:in
`test_should_count_manual_select_with_include'
     ./test/cases/../../lib/../../activesupport/lib/active_support/
testing/setup_and_teardown.rb:59:in `run'

   2) Failure:
test_add_index(MigrationTest)
     [./test/cases/migration_test.rb:82:in `test_add_index'
      ./test/cases/../../lib/../../activesupport/lib/active_support/
testing/setup_and_teardown.rb:59:in `run']:
Exception raised:
Class: <ActiveRecord::StatementInvalid>
Message: <"SQLite::Exceptions::SQLException: indexed columns are not
unique: CREATE UNIQUE INDEX \"key_idx\" ON \"people\" (\"key\")">
---Backtrace---
./test/cases/../../lib/active_record/connection_adapters/
abstract_adapter.rb:151:in `log'
./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:132:in `execute_without_counting'
./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:353:in `catch_schema_changes'
./test/cases/../../lib/active_record/connection_adapters/
sqlite_adapter.rb:132:in `execute_without_counting'
./test/cases/helper.rb:38:in `execute'
./test/cases/../../lib/active_record/connection_adapters/abstract/
schema_statements.rb:197:in `add_index'
./test/cases/migration_test.rb:82:in `test_add_index'
./test/cases/migration_test.rb:82:in `test_add_index'
./test/cases/../../lib/../../activesupport/lib/active_support/testing/
setup_and_teardown.rb:59:in `run'

of those only the first one looks like
http://dev.rubyonrails.org/ticket/11502
: the fix applied also needs to be applied to the sqlite workaround.
I'll bung all this in a patch (patches ?) if people are interested.

Fred
07245269d795e61e92a83447defd182f?d=identicon&s=25 Murray Steele (Guest)
on 2008-04-17 13:31
(Received via mailing list)
On 17/04/2008, Frederick Cheung <frederick.cheung@gmail.com> wrote:
> "developers".id  LEFT OUTER JOIN "projects" ON "projects".id =
> "developers_projects".project_id     WHERE (projects.id = 2)  LIMIT 1
>
> returns the selected column as id whereas with sqlite2 it returns it
> as "developers".id
>
[snip]
> I'll bung all this in a patch (patches ?) if people are interested.
>

So, that's pretty interesting, as in *my* version of sqlite3 (3.5.7)
running
the above sql was returning "developers".id not just id.  And thus I had
about 9 eager loading test failures, but applying your regex fix sorts
them
all out.  If you were to create a patch with just that regex fix, I'd
totally +1 it.

Although, it does make me wonder if there some change between different
versions of sqlite3 that cause my version and your version to react
differently?  Do we need to say something like this sqlite3 adapter
works
for this version of sqlite3?

Muz
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-17 14:20
(Received via mailing list)
On 17 Apr 2008, at 12:30, Murray Steele wrote:
> "developers".id  LEFT OUTER JOIN "projects" ON "projects".id =
> your regex fix sorts them all out.  If you were to create a patch
> with just that regex fix, I'd totally +1 it.
>
> Although, it does make me wonder if there some change between
> different versions of sqlite3 that cause my version and your version
> to react differently?  Do we need to say something like this sqlite3
> adapter works for this version of sqlite3?
>
I've got an older version (3.4.0). Updating sqlite3 caused me to get
those test failures on trunk too.
This does sounds like a bit of a nightmare waiting to bite people on
the butt. I'll write up a patch as soon as i have time

Fred
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-19 02:29
(Received via mailing list)
On 17 Apr 2008, at 13:19, Frederick Cheung wrote:
>>
> I've got an older version (3.4.0). Updating sqlite3 caused me to get
> those test failures on trunk too.
> This does sounds like a bit of a nightmare waiting to bite people on
> the butt. I'll write up a patch as soon as i have time
>

I wrote these up as
http://rails.lighthouseapp.com/projects/8994/ticke...
  &
http://rails.lighthouseapp.com/projects/8994/ticke...
  for any other sqlite users that are interested.

Fred
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-21 09:57
(Received via mailing list)
On 17 Apr 2008, at 08:24, Michael Koziarski wrote:

> adapters.   So sqlite, mysql and postgresql are the ones we're looking
> after.   The sqlite2 test failures are strange, is there anyone out
> there using it at present?  Would be good to fix this before 2.1 or at
> least document the missing functionality.

So, since you applied the patches I write for those issues sqlite3,
mysql & postgres are all happy, sqlite2 has a single failure that I'm
looking at. So far, I've left the schema files for the other databases
there (along with the code in the aaa_create_tables_test) to load
these (but turned off) on the ground that I wouldn't want to make it
difficult for someone fix any failures this causes in other databases.
Alternatively I could just blow it all away (after all that is what
source control is for). Opinions ?

Fred
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-21 10:20
(Received via mailing list)
On 21 Apr 2008, at 08:56, Frederick Cheung wrote:

>>> the
> So, since you applied the patches I write for those issues sqlite3,
> mysql & postgres are all happy, sqlite2 has a single failure that
> I'm looking at. So far, I've left the schema files for the other
> databases there (along with the code in the aaa_create_tables_test)
> to load these (but turned off) on the ground that I wouldn't want to
> make it difficult for someone fix any failures this causes in other
> databases. Alternatively I could just blow it all away (after all
> that is what source control is for). Opinions ?

Found out why the test was failing, turned out to be very simple:
http://rails.lighthouseapp.com/projects/8994-ruby-...

Fred
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-21 11:09
(Received via mailing list)
>  So, since you applied the patches I write for those issues sqlite3,
>  mysql & postgres are all happy, sqlite2 has a single failure that I'm
>  looking at. So far, I've left the schema files for the other databases
>  there (along with the code in the aaa_create_tables_test) to load
>  these (but turned off) on the ground that I wouldn't want to make it
>  difficult for someone fix any failures this causes in other databases.
>  Alternatively I could just blow it all away (after all that is what
>  source control is for). Opinions ?

I think it's all good to clear those out entirely, so just upload a
patch (using git format-patch on your branch) to wrap this up, and
I'll apply it.

--
Cheers

Koz
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-21 12:05
(Received via mailing list)
On 21 Apr 2008, at 10:08, Michael Koziarski wrote:

>> source control is for). Opinions ?
>
> I think it's all good to clear those out entirely, so just upload a
> patch (using git format-patch on your branch) to wrap this up, and
> I'll apply it.
>

There we go:
http://rails.lighthouseapp.com/projects/8994/ticke...

Fred
This topic is locked and can not be replied to.