Silent failure in test setup [BUG]

[dvnstrcklnd]

If you write a test setup function like so:

class ProtocolTest < ProtocolTestBase
  def setup
    add_operation
      .with_input('Valid Input Name', Sample.find_by_name('My Sample'))
      .with_property('INVALID Property Name', 1)
      .with_output('Valid Output Name', Sample.find_by_name('My Sample'))
  end
.
.
.
end

The 'INVALID Property Name' causes both the with_property and the with_output functions to fail silently. Even though the cause is obvious once you see it, it can be very difficult to debug because the missing output sample is more noticeable.

[bjkeller]

The root of this problem is that errors for FieldValues are captured silently. Specifically, Operation.with_property calls FieldValuer.set_property that does check whether the property exists, but captures errors in an attribute rather than throwing an exception: https://github.com/aquariumbio/aquarium/blob/ac7586cca2001c2b0fadbf7eb1421471eb75d2e1/app/helpers/field_valuer.rb#L72

Test failures are handled by exceptions, but it is not clear what the implications would be for changing this to raise an exception since this mechanism is used for other classes (e.g., Sample)

Another option may be adding a check at the end of ProtocolTestBase.run, but checking for operations with non-empty errors didn't work. This may require adding a predicate to Operation

[dvnstrcklnd]

So even the parts of the testing infrastructure that are "working" have underlying issues, and fixing these issues is complicated by the code in question being coupled to other parts of the app. Sounds like this is going to take a lot of work.

[bjkeller]

It turns out that particular method is only used for operations and almost entirely in the context of internal testing. What I was trying to do seems to work in that context, but not through the test controller.

So, I changed this method so that it raises an exception, and added handlers at calls to maintain behavior. There is a remote chance that this could mess with the designer, so I'm not merging/closing until that is verified. But it is available in the aquariumbio/aquarium:edge image at Docker Hub.

You can use this with aquarium-local by editing your .env file to set AQUARIUM_VERSION=edge (alternatively checkout the edge branch of aquarium-local and run ./aquarium.sh update)

BTW I used the existing messages, and they may lack some detail