Validation Engine Leaky Abstraction

[Castone22]

Currently writing a validator of any kind requires that the calling logic knows WAY too much about the internals of validation engine. It looks something like

def my_custom_validator
  validation_engine = @world.validation_engine
  validation_point = validation_engine.add_valiation_point('some validation point')
  validation_engine.enter_validation_mode unless validation_engine.validation_mode?
  validation_point.pass if(1+1 = 3)
  validation_point.fail('some fail message') unless(1+1 = 3)
  validation_engine.exit_validation_mode
end

My proposal is to add some methods to the validation engine that fully encapsulate most of this logic.

  def validate(message, fail_message)
    enter_validation_mode unless @validation_mode_on
    validation_point = add_validation_point(message)
    result = yield
    if result
      validation_point.pass
    else
      validation_point.fail(fail_message)
    end
  end

  ##
  # Same as validate except it will halt test execution if it fails.
  def validate!(message, fail_message)
    validate(message, fail_message)
    exit_validation_mode
  end

This would make writing the validator MUCH cleaner

def my_custom_validator_mk2
  @world.validation_engine.validate!('some validation point', 'some failure message') do
    1+1 == 3
  end
end

[greenarrowdb]

I understand what you're going for here and I agree with the sentiment that there is too much knowledge being exposed. Not sure the proposed solution encompasses the reasoning behind why we have 'validation mode' though. Validation mode was originally conceived so that we could do a bunch of checks on a page without having to fail the whole test at the first bad check (This was a major pain-point at the time). I'm worried that with this change that's going to get lost. Specifically, from what it looks like here validate won't work right unless you have a validate! call after it (so this still requires knowledge of how the validator works, but it's just less obvious).

Could we instead have a validate! that both enters and exits validation mode (for one offs) and some sort of multi-validate method where we could pass in a list of things to validate of some kind that would enter and exit validation mode around that list? Thoughts?

[Castone22]

yeah, i see no problem with that, the main reason i have validate and validate! was for soft and hard fails basically, the idea was every test should always end with a validate! (and typically validate! is the default you'll use.)

The only issue there is syntactic sugar, how would we want the body of multi-validate to look, i could make it a block wrapper like

multi_validate('Some optional high level idea') do
  validate(message, thing)
  things.each do |baz|
    validate(message, baz)
  end
end

At that point it's basically just rspec aggregated assertions, anything in this block does not cause a hard fail until block end.

def multi_validate(optional_message)
  enter_validation_mode
  yield
  exit_validation_mode
end

At that point, i can make validate raise an error if validation mode was off.

[greenarrowdb]

Let's try to find a time to chat about this one, I think we're moving in the same direction.

[Castone22]

I've been mulling it over and i'm thinking something like this would be best solution

  def perform_validations(msg = nil)
    msg = "Performing multiple validation: #{msg}" if msg
    msg ||= 'Performing multiple validations.'
    @world.logger.action(msg)
    enter_validation_mode
    yield
    exit_validation_mode
  end

@world.validation_engine.perform_validations do
  things.each do |baz|
    validate(message, fail) do
      baz == 'foo'
    end
  end
end

class CorePage
  def validate(message, fail_message)
    @world.validation_engine.validate(message, fail_message)
  end
end

Unfortunately for this convention to work and look like a proper dsl, we need to define validate on any class that would be calling it.

  def perform_validations(msg = nil)
    msg = "Performing multiple validation: #{msg}" if msg
    msg ||= 'Performing multiple validations.'
    @world.logger.action(msg)
    enter_validation_mode
    yield self
    exit_validation_mode
  end

If i defined this instead we could do something like

@world.validation_engine.perform_validations do |ve|
  things.each do |baz|
    ve.validate(message, fail) do
      baz == 'foo'
    end
  end
end

Though that feels clunky to me for some reason...