rails / thor

Thor is a toolkit for building powerful command-line interfaces.
http://whatisthor.com/
MIT License
5.12k stars 553 forks source link

🌈 Allow options to be set back to defaults #713

Closed timothysmith0609 closed 4 years ago

timothysmith0609 commented 4 years ago

Why do we need to be able to reset options?

674 introduced a subtle bug that occurs if a Thor command is invoked multiple times during its lifetime (this will most likely only occur when running test suites). The failure mode in this case is that the previously parsed options are not cleared, and instead persisted to the next invocation of a Thor command.

For a concrete example: the krane tests were failing due to filename args being persisted and accumulated across tests.

The offending lines are options.rb#L136-L140, where we merge, instead of overwrite when repeatable is true.

The most straightforward solution seems to be adding this escape hatch that will reset options back to defaults and allow for reparsing from scratch. Let me know if that's a satisfactory approach.

An alternative solution could be to always reset options back to defaults every time parse is called. I chose not to do this since the existing test indicates that was not desired behaviour. This approach has the benefit of taking the onus off the caller to be mindful of clearing options.

timothysmith0609 commented 4 years ago

Closing as the actual error was in our application (we were calling CLI methods at the class, not object, level)