Closed jthomperoo closed 5 months ago
thanks! i was going to check it out before i merged all in, but appreciate it :-)
yeah i just ran main locally and they all seem to be passing there fwiw! regardless, i'm sure it's just minor tweaks and i appreciate all the effort with this 🍻
Hi, I noticed the CI was failing, so I've attempted to fix that under commit f9ec13d.
Once I had the CI running again there were two unit tests failing (
test_in_operator
andsimple_in_operator_for_string
), which were both failing due to filtering by enums using the 'in' filter.These look like they had been failing for a while, I couldn't pinpoint exactly when they started failing though, I was getting xUnit errors when trying to run the tests on older commits, probably something I haven't set up on my machine though.
The problem seemed to be when you filtered by an array of enums, e.g.
birthmonth ^^ ["January", "March"]
, it recognised the type as an enum, but just tried to run the array string directly throughEnum.Parse
. I've updated the logic to detect if it is an array in the same way arrays of strings are detected in the FilterParser, and then individually parse each Enum entry. This looks like it fixes it, but still required the unit tests to be updated to reflect the new types. This was all done under commit 783b387.Finally I just wanted to make sure I hadn't broken any functionality with any of this, so I've added some integration tests to cover some
Enum
scenarios under commit 662ff59. The tests compare enums, but also against a faked title just to avoid getting a whole load of other entries with matching enums, not sure if this is the best way to do it, happy to change it if there's a better approach.These changes around Enums will probably cause conflicts with my other merge #28, so I'll have to update that if you accept this pull request.
Hope these changes all make sense!
Thanks.