"pending" feature to lint factories that are expected to be invalid

[jasonkarns]

Problem this feature will solve

In a legacy codebase that has a large suite of factories, and a large chunk of them being invalid, it can be difficult to incrementally fix the factories. The factorybot linter is fantastic for discovering invalid factories. But, as known with other forms of automated tests, the linter is only useful when it can be kept green. That is, when adding the linter to an existing suite of factories with dozens or more invalid factories, the known-invalid factories must be initially ignored. (Similar to xit in rspec.) As the factories are incrementally addressed, they can be removed from the "ignore list" and subsequently included in the automated lint task. However, should a factory become "valid" while ignored, it must be removed from the ignore list in order to prevent it subsequently becoming invalid again!

Therefore, a more robust approach for validating the factories would be to have two lint invocations:

1. linting the expected-valid factories; failing if any are invalid
2. linting the expected-invalid factories; failing if any are valid

The latter represents the same behavior that RSpec's pending helper accomplishes. That is, the spec is run and is expected to fail. But if it succeeds (unexpectedly), it is recorded as a failure so that it can be removed from the pending list.

Desired solution

Ideally, the FactoryBot.lint would expect another option: pending: true or similar that would fail for any given factory that is valid.

Usage:

invalid_factories = [...]
FactoryBot.lint (FactoryBot.factories - invalid_factories)
FactoryBot.lint invalid_factories, pending: true

The ideal output would then adjust the messages to indicate that any factories that were not invalid, were in fact valid (unexpectedly); essentially negating the error messaging.

Alternatives considered

Additional context

[sej3506]

We agree this feature could be useful for maintaining a large suite of factories. We have a few considerations on the suggested approach.

• Requiring a list external to the actual factory files feels like a hurdle for the developer using the feature.
• If a factory is deleted it will still be invalid until someone remembers to remove it from the invalid factories list.
• Implementing this on the linter would require inverting the logic, which is probably low effort but the output is a string of errors. The process of a developer manually updating an external list vs the factory itself seems like an unnecessary additional abstraction.

Based on the example of Rspec's pending provided, what do you think about this approach? We could add an option to the factory definition that marks it as pending (or some other term). Pending here would mean that when a linting check happens, an error would be added if it does not throw a linting error. This would remove the obfuscation caused by an external list because it would live closer to what we are trying to validate with the linter.

	Valid factory	Invalid factory
Factory default	✅Passes linting	❌Linting error
Factory pending	❌Linting error	✅Passes linting

[jasonkarns]

@sej3506 I think keeping the pending metadata close to the factory definition is a fantastic idea! On top of all the considerations you listed above, it's also a great visible marker to developers when they view a factory that it still needs attention. Love it!