Rewriting Queryable#query specs to test behavior

[no-reply]

Specs for Queryable#query tested internal implementation rather than
behavior, this caused issues for classes mixing in Queryable, but
delegating #query to some collaborator. See,
e.g. ActiveTriples/ActiveTriples#169.

This tests the contract directly, instead.

---

Before merging, note that these tests are cribbed from #query_pattern and #query_execute. Both are protected, but it's not clear to me whether they are part of the required interface for Queryable (i.e., should they be private?).

Depending on the answer, we should either remove the tests, or merge them into shared examples to deduplicate.

[gkellogg]

The introductory comment to RDF::Queryable says:

Classes that include this module should implement a #query_pattern method that yields {RDF::Statement RDF statements}. Classes may also implement an optimized #query_execute method that yields {RDF::Statement RDF statements}.

So, they should be protected.

[gkellogg]

For ActiveTriples using the delegate pattern it seems that delegating query to RDF::Graph is reasonable.

For classes which extend Queryable, they should implement query_execute and query_pattern, which might just be shims for redirect to the RDF::Graph implementations in this case, but delegating query seems to do the trick.

Otherwise, I'm not sure if moving the query_pattern/query_execute behavior under the query specs is necessary, but I don't see it causing any harm. It could be extracted into common behavior which is used for both query_pattern/execute and query.

[no-reply]

I guess the way I'm seeing this is that Queryable offers a default #query, meeting a fairly complex set of requirements, if the implementer can provide two fairly simple methods in the protected interface. This is good.

The specs, however, focus on testing the protected methods, and test the public one only indirectly. So for RDFSource we have an otherwise nicely working Queryable class (getting much of its behavior via extension), but are failing the tests because of an implementation detail; that #query is delegated instead of relying on protected methods. My feeling is that the shared examples should test the desired behavior, rather than adoption of the mixin's convenient structure.

I allow I may be missing something here. That the methods are protected rather than private suggests that other instances may be intended to use them; but I don't see where that actually happens or why they wouldn't just use #query with the relevant argument patterns.

Hopefully that helps clarify where I'm coming from. :)

[gkellogg]

That they're protected says that it is allowed/intended for concrete implementations extending Queryable to implement them, and the specs insure that such implementations perform as expected. However, it's reasonable for concrete implementation to implement query instead, so having the behavior tested for both #query_execute/pattern and #query makes sense.

[no-reply]

While thinking about the above, I considered a couple of things:

• Maybe the deleted specs in the PR should be moved to mixin_queryable_spec.
• Perhaps the right approach to the shared examples is to test #query_execute & pattern if they are present. Then it would be responsible for a class like RDFSource to either undef them, or to provide a conforming implementation.

[gkellogg]

👍