Project

General

Profile

Actions

Bug #4434

closed

[API] Test suite should not depend on a particular database collation config.

Added by Tom Clegg over 9 years ago. Updated over 9 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
Story points:
-

Subtasks 1 (0 open1 closed)

Task #4435: Review 4434-collationResolvedTom Clegg11/07/2014Actions
Actions #1

Updated by Tom Clegg over 9 years ago

  • Status changed from New to In Progress
Actions #2

Updated by Tom Clegg over 9 years ago

  • Category set to API
  • Assigned To set to Tom Clegg
Actions #3

Updated by Tom Clegg over 9 years ago

4434-collation @ c1f7fd8

Also consolidated the two nearly-identical tests into a loop.

Actions #4

Updated by Tom Clegg over 9 years ago

It seems it's not quite that easy to make the Rails application agree that "abc" <= "BCD" in the same cases where the database thinks so.

Rather than wade into localization, perhaps we could restrict the test to only the collections whose first letter is (1) lowercase a-z, and (2) distinct -- i.e., filter for ^[a-z], sort uniquely on first character. Ruby and every useful locale setting should(?) agree about the correct sort order of the resulting set.

Actions #5

Updated by Tom Clegg over 9 years ago

4434-collation at fbd2326

Actions #6

Updated by Radhika Chippada over 9 years ago

Review feedback:

  • If I had more than one collection starting with the same letter, for example if I were to add one more collection with the name 'baz_file2', only one of them is retained in "sorted_names" and the rest of the test. I think this is not good. I think we can get rid off "do |name|" in the following.
      reliably_sortable_names = sorted_names.select do |name|
        name[0] >= 'a' and name[0] <= 'z'
      end.uniq do |name|
        name[0]
      end
  • Tests now pass for me. They pass even after I delete the "do |name|" logic.
Actions #7

Updated by Tom Clegg over 9 years ago

Radhika Chippada wrote:

Review feedback:

  • If I had more than one collection starting with the same letter, for example if I were to add one more collection with the name 'baz_file2', only one of them is retained in "sorted_names" and the rest of the test. I think this is not good. I think we can get rid off "do |name|" in the following.

The purpose of .uniq do ... here is to avoid testing whether "bar" <= "bBr", which is true in some collations but not others.

Perhaps a better test is to ensure that the order is reversed between "asc" and "desc"? If the database thinks "bar_file2" <= "bar_file", and consistently sorts in both directions according to that rule, who are we to argue?

I noticed something else by misreading your comment at first: Experimentally, ['bar','foo','bar'] & ['bar','foo'] == ['bar','foo'], so if we receive two collections with the same name "baz_file2", one of which is out of order, we might not detect it. So I've changed the &= statement to sorted_names.select! so it preserves ['bar','foo','bar'].

  • Tests now pass for me. They pass even after I delete the "do |name|" logic.

Removing do |name| stuff from this test just makes it more dependent on your database having an ASCII-like (case-sensitive) collation or the fixtures not happening to expose the difference between DB and ASCII collations. But it doesn't necessarily fail right away. Making a test to demonstrate that the sort/uniq stuff is necessary for your database collation might be more trouble than it's worth. (But my guess is that if you add a collection just like bar_file but with name="bBr_file", the "uniq" part will start to become necessary for the test to pass.)

The more important question is: Is it possible for this test to pass when the database sends stuff in the wrong order? For example, if I remove the " #{order}" part of the get statement to simulate a sorting failure, the test fails the _desc case. Does it do that for you too?

(The goal is to make the test stop depending on the database collation, without making it unable to detect any sorting failures at all... Detecting every sorting failure would require determining what the database collation is supposed to be, and replicating that collation in Ruby, which is way too much to fit in this bugfix!)

Now at 0198b49

Actions #8

Updated by Radhika Chippada over 9 years ago

Tom, in that case, I am ok with what the test is doing. Please go ahead and merge. Thanks.

Actions #9

Updated by Anonymous over 9 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:5141c3ee23e89696773e227a93236ef2a51543c2.

Actions

Also available in: Atom PDF