Breaking changes on ServiceActor::Result `failure?` after dropping `ostruct` ???

[stephannv]

My tests started to fail after upgrading to 3.8.x because changes on Result.

My code before 3.8.x:

allow(MyActor).to receive(:result).and_return(ServiceActor::Result.new(failure?: true))

result = MyActor.result
result.success? # => false
result.failure? # => true

After upgrading to 3.8.x:

allow(MyActor).to receive(:result).and_return(ServiceActor::Result.new(failure?: true))

result = MyActor.result
result.success? # => true
result.failure? # => false

I think what caused the problem was this line: https://github.com/sunny/actor/commit/c73d688c0a11a0f41b40e54f6e18e2c95ed51db4#diff-6d7487eb2415132d6b0752596159f369f60fc82446b1a372ea0d5841fed7678cL25

Fixing things on my side is easy, I have to change all ServiceActor::Result.new(failure?: true, ...) by ServiceActor::Result.new(failure: true, ...), but I'm reporting here because it could be a problem for more people.

[sunny]

Oh indeed, we didn’t think of people who would intentionaly create results with failure? as a key. This is now fixed in v3.9.0.

Thank you for bringing this up @stephannv 🙏🏻

[stephannv]

Oh indeed, we didn’t think of people who would intentionaly create results with failure? as a key.

If you consider failure? hash option was a private API and shouldn't be used, you could keep it breaking or you could deprecate it to remove on 4.x

[dmkl]

Passing 'failure?: true' is very handy in specs for testing expected error handling. Please don't remove it.

[stephannv]

Passing 'failure?: true' is very handy in specs for testing expected error handling. Please don't remove it.

It wouldn't be removed, it would be changed from failure?: true to failure: true. It isn't common to use hash keys using ?

[sunny]

Yeah I wouldn’t mind gently deprecating it but I think it’s also fine to accept both failure?: true and failure: true as keys.