shimmur/checker_cab

an extraction of assert_values_for

checker_cab's Issues

Add example output to readme

It would be great for our readme to show examples of the failure output.

add option to fail if both values are nil

It can happen sometimes that the test setup is incorrect and the code under test is similarly bad, resulting in both values for a key being nil. As the library is currently written, this will pass. It should stay the default behavior, I think, but we should look at adding an option that will raise if both values for a key are nil.

RFC: field name translations

Been thinking of ways that assert_values_for doesn't quite reduce effort and one of the more common places is when the two map-likes (map, struct, ecto schema) have different names for the fields, but the data is the same. An example might be when a database struct, Thing, has an id field, but a system event might call it something like thing_id.

As with the list assertions, I think the biggest challenge is the api/dsl to make it clear to someone unfamiliar with the library what is going on.

The fields list will need to be the source for the starting key. But then, is it assumed that the fields passed to fields are always a match to the fields on expected? If so, I think we could possibly get away with a less verbose, more implicit dsl, but I'm not sure that that is always the case. If not, we should be more explicit.

Here is a sketch of what it could look like. I would love feedback or ideas that aren't presented here at all.

      assert_values_for(
        expected: quote_request,
        actual: quote,
        fields: fields_for(OmniWeb.Schema.QuoteRequest),
        field_name_translations: [
          # explicit
          origin_postal_code: [actual: :origin_zip],
          destination_postal_code: [expected: :destination_zip, actual: :destination_zipcode],
          # implicit
          pieces: :freight_pieces, # assumes field is from expected and that this is on actual?
          weight: :freight_weight
        ],
        skip_fields: [
          :selected_common_pickup_accessorials
        ]
      )

Error results should show all mismatching fields, not just the first one.

Current behavior is that the first field to mismatch raises an error and the output only shows that one field. It would be great to improve the output so show all mismatching fields and only raise at the end.

This should be possible without changing the api and keeping it backwards compatible.

Support Elixir 1.14.x and OTP 25

We should be able to add another entry in the workflows to add these versions to CI.

RFC: proposal to add assertions for list of map-likes

I have been playing around with the idea of extending CheckerCab to use the same assertion system for lists of map-likes (map, struct, ecto schema). Overall, I don't think it's too hard, but coming up with a clear API is the challenge. I have thrown out a couple passes on what it could look like, but am open to new ideas as well as different terms.

option 1 - new function

The main question is whether or not it should be a new function or an extension of the behavior of the existing function. I'm leaning towards a new one, though naming, as usual, is hard.

      assert_values_for_in_list(
        expected_list: {[thing_1, thing_1], :string_keys},
        actual_list: return_value, # is list in all the examples
        ordered: true,
        fields: fields_for(Thing)
      )

      assert_values_for_in_list(
        expected_list: [thing_2, thing_1],
        actual_list: return_value,
        ordered: false,
        identifier_key: :id, # this would have to be required when ordered is false
        fields: fields_for(Thing)
      )

option 2 - add an optional key to existing function

Alternatively, we could add a new key to the existing function. I'm concerned that this might leave it hard to distinguish the behavior.

     assert_values_for(
       type: :ordered_list, # default would be something like :single_map, leaving this backwards compatible
       expected: [thing_1, thing_2],
       actual: return_value,
       fields: fields_for(Thing)
     )

     assert_values_for(
       type: :unordered_list,
       unique_identifier: :id,
       expected: [thing_2, thing_1],
       actual: return_value,
       fields: fields_for(Thing)
     )

I would love feedback and any other suggestions.

allow nested keyword lists for `:skip_fields`

It would be great to be able to annotate why certain fields are being skipped. I think the easiest way to do this is code would to be able to do something like:

assert_values_for(
  expected: user_params,
  actual: user_from_db,
  fields: fields_for(User),
  skip_fields: [
    db_generated_values: [
      :id,
      :inserted_at,
      :updated_at
    ],
    keys_different_from_params: [
      :phone_number,
      :some_random_thing
    ]
  ]
)

This is doable with minimal changes, I think (flatten and take the values?) and will allow the calls to read as better documentation and may even highlight to the coder which keys should be intentionally tested after.

Should work with a list of string field names

Given that this tool originated to compare web inputs and the resulting database data, it would make sense if we could use parsed json as the source for the expected fields, which means string keys.

Looking at implementations, it felt like either ‘assert_values_for/1’ for ‘fields_for/1’ could be updated, but I think that ‘assert_values_for’ is the right solution, as using ‘fields_for’ isn’t required.

Under the hood, I think we just convert fields to atoms. just need to make sure we test edge cases around skip_fields as well.

Fix typo in test output.

We have an issue with the main error message:

  defp error_message_for(missing, mismatched) do
    ["There were issues the comparison:", error_message_for_missing(missing), error_message_for_mismatched(mismatched)]
    |> Enum.reject(&(&1 == ""))
    |> Enum.join("\n\n")
  end

See the file here

The error string should read "There were issues with the comparison:"

support Date conversions and not just DateTimes

we missing converting iso8601 date string. we should support them, too.