[go: up one dir, main page]

Skip to content

Validation of repository_storages_weighted is spread across the class and concern, difficult to test

The following discussion from !170457 (merged) should be addressed:

  • @drew started a discussion:

    So the undercoverage on 705 seems to be legitimately never run, but it seems legitimately unreachable because of the coerce_repository_storages_weighted implementation:

      def coerce_repository_storages_weighted
        repository_storages_weighted.transform_values!(&:to_i)
      end

    which is run before validation by ApplicationSetting.

    This validation that lives in ApplicationSettingImplementation is like a... backup validation? That would run if ApplicationSetting wasn't written the way it was? But ApplicationSettingImplementation doesn't seem to have it's own spec.

    This is a kind of messy blend of responsibility shared between the model and the concern, and we should probably sort it out to make it easier to understand which code is responsible for what. I'm going to make a follow-up about this and apply pipeline:skip-undercoverage to let this refactor pass 👀