Optional sorbet-runtime support for JobIteration::Iteration interface validation

[paracycle]

Methods that have Sorbet signatures are wrapped by sorbet-runtime so that they can be type-checked for correct params/return-value at runtime. However, that wrapper does not and cannot relay the whole information about the original method like arity or parameters. (Ref: sorbet/sorbet#2643)

The workaround is to access the original method from the signature if we detect Sorbet is activated and the method in question has a signature.

This PR abstracts the method parameter extraction into a separate method in order to carry out that workaround and adds a test to ensure that interface validation works even when the interface methods are wrapped by Sorbet.

[gmalette]

Thanks @paracycle!

[kirs]

Thanks! Is there any other things we can make to Iteration jobs typed? Can we use an actual Sorbet interface to enforce all subclasses to respond to right methods and signatures?

[gmalette]

Can we use an actual Sorbet interface to enforce all subclasses to respond to right methods and signatures?

Unfortunately they only work if the class implementing the interface is also typed.

HOWEVER. IIRC the check for the cursor parameter happened when we redesigned the API of Jobs::Iteration to thread the cursor, instead of the framework fully managing a narrow set of Iterators. Have all the occurrences been fixed? What would you think of just removing the check?

[kirs]

Unfortunately they only work if the class implementing the interface is also typed.

Can't we make ApplicationJob in Core typed by default and include the interface?

HOWEVER

The framework only allows def build_enumerator(params, cursor:) right now.

Maybe I'm misunderstanding you, but the whole valid_cursor_parameter? check is mostly necessary to make it more user friendly in case someone forgets to put the kwarg. We want to detect that early so they don't spend too much time fighting with code not working due the missing arg. Maybe that's too defensive.

[gmalette]

The framework only allows def build_enumerator(params, cursor:) right now.

Cool! I remember correctly that we added this specific check during the transition between build_enumerator(params) -> build_enumerator(params, cursor:)?

We want to detect that early so they don't spend too much time fighting with code not working due the missing arg.

Oh. To check what error message would be produced I commented the check, picked a random task, and removed the cursor argument. Ran the tests, and got

ArgumentError: wrong number of arguments (given 2, expected 1)

Unfortunate :(. Maybe with 2.7 that error would be better?

Can't we make ApplicationJob in Core typed by default and include the interface?

We can, but that doesn't force the specific maintenance tasks to be typed so I don't think it solves the problem.