Giter VIP home page Giter VIP logo

Comments (11)

kennycason avatar kennycason commented on August 22, 2024

As an update, It may note have been an update from version 15 to 16 (as we originally weren't specifying versions so I am a bit unsure). However, I noticed that in 15 expect_json_types is throwing a no method found error, suggesting that this must have been a change in 16? i.e. expect_json_types didn't exist < 16, so I had to have been using 16, and expect_json's implementation change in 16 midcourse?

~~Sorry for the confusion on my part. I just happened to notice inconsistent behavior in unchanged tests :)~~~

from airborne.

kennycason avatar kennycason commented on August 22, 2024

Well, some good news. I found the issue with expect_json_types, it was a null value returning: NoMethodError: undefined method 'include?' for nil:NilClass, of which caused me to hastily reach a false conclusion. So my only issue is around the breaking update with expect_json

from airborne.

sethpollack avatar sethpollack commented on August 22, 2024

Yes, sorry, the change was introduced in #52, and now I think that expect_json and expect_json_types both work differently.

The plan is to add options which can be configured in the Airborne.configure block or passed into individual tests.

The options will allow you to test:

  1. That all keys in the expected hash are in the response.
  2. That all keys in the response are in the expected hash.

So if you only want to check the keys that are in both, you can turn both these options off.

from airborne.

kennycason avatar kennycason commented on August 22, 2024

Yeah I think something like that would be useful. Probably instead of configurations I would recommend two separate functions. I definitely like having the option of both! :)

from airborne.

ivan-kolmychek avatar ivan-kolmychek commented on August 22, 2024

@kennycason actually, we'll need four functions to cover all the possible combinations provided by two customizable variables (variables per value ^ variables amount):

  • false, false - check only intersection
  • false, true - raise on extra key in response
  • true, false - raise on missing key in response
  • true, true - expect exact match

So, config variables do look like the most painless way to do it. Also, if we introduce just another variable (let's say, strictness or something like that) you end up with eight (2^3) functions, and so on, and so on.

So, if we will ever have 10 variables to controll the behavior of the function, would you like to have 1024 functions - one for each possible behavior?

I'm pretty scared of possible names for these functions also. ;)

from airborne.

kennycason avatar kennycason commented on August 22, 2024

"So, if we will ever have 10 variables to control the behavior of the function" - At the point you have 10 variables controlling the behavior of a function you should seriously consider breaking up the concerns of the function. For that matter, I'd wager that if we have > 2 or 3, the function is on a path for getting overly complicated. I don't think people want to constantly have to refer to manuals/documentation to figure out how to use a basic assert.

I am interested in some of the other "strictness" variables that you can imagine. case sensitivity? I'm not super sure, but they seem like they are likely very rare cases and I would poll your user base and/or examine usage. (I am ignorant of these details)

For me I personally only cared about two cases, and have continued only caring about those two: (obviously I'm aware others my differ in opinion here)

  • assert that at least the provided keys exist in response. (assert a subset of the format)
  • assert that the provided keys map directly to, 1:1, the response. (assert exact format)

As a general API design, I tend to prefer simple.

from airborne.

ivan-kolmychek avatar ivan-kolmychek commented on August 22, 2024

@kennycason

At the point you have 10 variables controlling the behavior of a function you should seriously consider breaking up the concerns of the function.

Yes, I agree with that - I do prefer simple methods with clear responsibilities, possibly even pure functions without any side-effects, but it's all about tradeoffs. Maybe you're right and with two variables it's worth to create four separate methods, but I'm not sure it's a sustainable approach for gem which aims to be a " framework" (judging by a Readme) - see next point.

I am interested in some of the other "strictness" variables that you can imagine. case sensitivity? I'm not super sure, but they seem like they are likely very rare cases and I would poll your user base and/or examine usage. (I am ignorant of these details)

I'm pretty sure somewhere in the life cycle of the library someone will come and say "Hey, you know, we may need X for case Y in here", with someone other "yes, me too, I would even to a pull-request".

As a general API design, I tend to prefer simple.

I prefer simple too, but simple does not mean you must remove the ability to build complex things with these simple building blocks, so I prefer simple, yet composable. ;)

Take a look at #74 and rspec-expectations matchers - they have simple matchers with powerful yet simple interface which allows you to say whatever you want to say.
For instance, take a look at includes, which acts like

expect([1, 2, 3]).to include(2, 3) # pass
expect([2, 3]).to include(1, 2, 3) # fail

here, we have one simple matcher, yet, by providing the interface which allows us to set both actual value and expectation, it covers both cases of partial match.
Also, we have chainable matchers and composable ones, so, you can say

expect("this car", "this chair").to all(start_with("this").and end_with("r"))

or

expect(obj).to respond_to(:bar).with(2).arguments

I think, that's good to aim for something like that, don't you agree?

from airborne.

kennycason avatar kennycason commented on August 22, 2024

"Maybe you're right and with two variables it's worth to create four separate methods" - No, I agree, I don't want a million functions either. :) I mainly just criticizing the 10 parameter argument as I thought it was a bit on the hyperbolic side, and pointing out that for my projects I only needed two and not four functions given the stated examples. And I didn't want to be passing in myAssert(expect, actual, true, false) or something like that as it's "ugly". I don't doubt people will have special use cases.

One thing that I like about the library so far is that it is terse, flexible, and easy to read.

Having a configuration file define how the function behaves so that you can type myAssert(expect, actual) and it will do different things based on said configuration, is not easy to read because it won't be obvious what the function is actually doing, especially months later. Another note, myAssert(expect, actual, true, false, true) is also not super easy to read, best case it's not pretty.

By externally configuring I would be locking that function's behavior for the whole project, when really I may want to use it differently in different places.

NOTE I want to note that my concerns original arose from the mentioning of using an configuration file to determine the behavior of a function. I would rather have 1) a function with parameters, or 2) multiple functions, over having a function who's behavior is defined by a configuration file, and then set for the whole project.

from airborne.

ivan-kolmychek avatar ivan-kolmychek commented on August 22, 2024

@kennycason ok, sorry, it looks like I misunderstood you earlier.

Yes, I agree, that making the method params implicit (configuration) is a kind of antipattern, as it hides the complexity of a function.

I would like this functionality to be incapsulated in an object (actually, RSpec matchers are objects, if I remember correctly) and a chainable matcher defined for it, so, if you need some specific functionality, you can chain it in readable way or, if you want to use it outside of RSpec, you can instantiate a matcher by yourself and define specifics by chaining its methods.

from airborne.

hcarreras avatar hcarreras commented on August 22, 2024

For future googlers: I had the same error because I specified a type which doesn't exists (:hash_or_null).

from airborne.

sethpollack avatar sethpollack commented on August 22, 2024

This is fixed now. I added the configurations.

from airborne.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.