gnecula / bond Goto Github PK

This is more of an idea for extending Bond.

I am in the middle of writing tests for a complex REST api interaction with multiple dependent round trips. It is a pain to collect the mock results. I keep going back and forth between the production system and the test, trying to get to the response to my requests in a debugger to save them to a file so that I can use them in mocks.

It just occurred to me that we are in a unique position to use spies to actually build the mocks. If you somehow turn on spying in production code, then you can use those observations as mock results.

Other mocking systems that are not coupled with spying may not be able to use replay. I am wondering how exactly to do this.

Currently Java has mocking capabilities through PowerMock, but they require a sizable amount of ugly setup code on each test (see http://necula01.github.io/bond/jbond/bond/spypoint/BondMockPolicy.html). There may be ways to reduce this; we should investigate further.

Users pointed out that if you pass a result function that throws exceptions it is not very clear what went wrong. I do not have a reproduce scenario for this.

The way observations are named by default when using Bond with RSpec is the entire test description, which is often very verbose. Some of the current bond_test observations (which I think are reasonable length for rspec) are over 100 characters, which exceeds the character limit for filenames to be included in a gem.

Something else needs to be used, though it's not really clear what would be best suited. Imposing a 100-character limit may be reasonable, but could result in a lot of overlap. Something like a 90-character limit plus some sort of unique string at the end (e.g. a hash of the full description) could resolve that, but isn't very human-friendly. Those are my only ideas for now, but I'll keep thinking about it.

While looking at some of the uses of Bond I realized that sometimes you want to spy some intermediate values in production code. With the new design we have the option to break the function there (ugly), or to create an empty function with a spy_point annotation.

I'd say we allow bond.spy in production code. All we need to do is to make sure it does return immediately if not in test.

Not all spy point use agents. If you do not have an agent, the spy point name plays only a documentation role. Sometimes I just want to spy "x". I would like to be able to write:

bond.spy(x = x)

instead of

bond.spy("my point", x = x)

Possible implementation

• Add a separate spy_anon (for anonymous spying). This will work well in all languages, but I would rather not create another entry point just for this.
• Make the spy_point_name an optional keyword argument. If not present, then it is anonymous. This works well in Python (at the extra cost of having to type spy_point_name='foo'). We'd have to do something else for Java.

I do not quite like how formatters work. Right now, they modify the observation dictionary in place. This makes it easy for the formatter, because you do not have to copy the observation, but has unpleasant consequences:

• the side-effects in the observation are visible to downstream functions, but not to filter and ignore functions, which are processed before
• the side-effects make it impossible to write one-line formatters using 'lambda' (but that is really a weakness in Python). Even without side-effects it would be hard to write one-liners.

What I would really like is to specify places in the observation where I want to make some changes. For example, I want to split the lines for key1[].foo, or I want to replace all strings that match a date in key2.dates[].

I do not understand this safe import. Does this allow "if bond.testing" ? Why do we need the DUMMY_BOND name, can't we just assign to bond directly?

begin
require 'bond'
rescue LoadError
module BondTargetable
DUMMY_BOND = Class.new { def method_missing(meth, *args); end }.new
def self.included(base); base.extend(BondTargetable); end
def bond; DUMMY_BOND; end
end
end

Right now it seems that the reconciliation too aggressively ignores whitespace. It seems to consider these as matching:

"__spy_point_name": "AnnotationTests.annotated_class_method",
"__spy_point_name": "AnnotationTes ts.annotated_class_method",

which is clearly not correct

pybond has code that is specific to unittest. Refactor that code with a bridge, so we can easily add support for other testing frameworks.

Right now if you change the observation directory, on the next run, all of your tests will pass without warning or anything. It would be really easy to accidentally change the observation directory, break a bunch of things, run the tests and have them succeed, and then check in that code. It's kind of an obscure situation but I kind of just almost did it to myself (because I was fiddling around with the code that sets the observation directory, broke it, and all of my tests were passing).

Not sure if we need to do anything about this (beyond maybe a comment in the documentation), but I think it's something to consider.

It seems that the terminology "merge" is not really appropriate. I think that

BOND_RECONCILE=accept is a better terminology.

What do you think?

In the old Bond I have a custom serializer that the json module invokes when it cannot serialize the object. Then I look for a special method toJSON, and if I find it then I serialize. What should we do in the new version?

Right now on test failure, we do not invoke reconcile, and we cannot see the differences.

We will add another parameter to bond_reconcile:
--no-save="Test failed"

In the no-save mode, you can see differences, but no merging is enabled (kdiff3 used in diff-view mode only).

If a spy_point has multiple agents set, all with doers, and one of those has a result, what's the expected behavior? I think we need to consistently decide if all agents are applied, or only the most recent one that matches. The two consistent directions I see: (1) More recent agents completely replace older ones (unless they don't meet the matching criteria) and thus only the most recent matching agent does anything (gets its doer called, has its ignore checked, etc). (2) All agents coexist but some have higher priority. All of their doers are run, all of their ignores are checked (but more recent ones take precedence), and then a result is found.

The getting started documentation lists a Maven dependency for Bond that can't be found. Searching for Bond also shows nothing.

Current way that we exclude the self and cls parameters is hacky. Need something better

In Ruby, it's pretty common to pass a block as an argument to a method. Should we include this in our observation? If so, how? The blocks can be pretty important to the program's execution but I see no easy way to include them in observations.

Is there a license you usually use for open source / one we're supposed to use because of Berkeley? Started thinking about this because RubyGems is mildly unhappy if you don't specify a license.

Conviva people pointed out that there is not reference description of reconciliation. E.g., how is BOND_RECONCILE used, what are the options, ...

We could make Bond more generally useful if it supports a plugin mechanism.

One possible design would be to allow the specification of a plugin object in settings. The plugin can override functions that allow somebody to change how Bond does things.

One use case that came to mind was to add a "before_reconcile" entry point that can pre-process the observation file before comparing it with the reference. I am sure that we will find other use cases (e.g., before_test, after_test, ...)

Writing the heat_watcher test has made me realize that the ignore option is really nice for situations where you want to mock but not spy (in the heat_watcher example, I really don't want to be observing all of the calls to get_current_time) but this isn't possible at the moment.

RSpec doesn't really have the concept of a test name. It refers to specific cases using a description, e.g. "Bond does something useful". What should we use as the test name, then? This is especially tricky since we're currently using the test name as the file name for the observations. We can strip out characters from the description of the test that aren't filename-friendly, maybe replace spaces with underscores or something, and use that? Or we can force users to specify a test name. Not sure what's best here.

I noticed the following:

bond.testing vs bond.TESTING
:agent_result_none vs bond.AGENT_RESULT_NONE
:agent_result_continue vs ...

It is not most important to keep the APIs identical, but we have to think how will be take advantage of join documentation or tutorials.

I understand python-tk is being used to create the dialog window when bond reconciliation is set to dialog, but python-tk is a huge library that I don't want in production. As-is it is slowing down my CI builds substantially.

Can this reconciliation method and dependency be removed?

Since we cannot use "return" as a parameter key in the agent, we use "result". We should then change the terminology to be consistency: spy_result in the spy_point and .result as the spy point name,