Skip to content
This repository was archived by the owner on Sep 16, 2019. It is now read-only.

BooleanHandlerIntegrationTest - #112

Closed
thiemowmde wants to merge 1 commit into
masterfrom
dvintegration
Closed

BooleanHandlerIntegrationTest#112
thiemowmde wants to merge 1 commit into
masterfrom
dvintegration

Conversation

@thiemowmde

Copy link
Copy Markdown
Contributor

This is a framework (yes, I use subclassing) that (finally) actually tests the DataValueHandlers query capabilities. More to come after this got reviewed and merged.

@JeroenDeDauw

Copy link
Copy Markdown
Contributor

-1 on the inheritance thing. Basically in all cases where I ever did this or let it go through review I ended up regretting it. If you want I will do a pass on this to get rid of that.

@thiemowmde

Copy link
Copy Markdown
Contributor Author

Not sure why you want to waste your and my time here. I spend two hours rearranging my new code to make it nice and readable and reusable. I created a test framework for classes that all depend on the same base class. What's wrong with that? I expected questions about the method_exists bit, but stating that this code is bad because other code that misuses inheritance is bad is silly.

@JeroenDeDauw

Copy link
Copy Markdown
Contributor

Indeed, I'm intent on wasting our time and being silly. Great you got that.

@thiemowmde

Copy link
Copy Markdown
Contributor Author

You realize that I don't see a way forward in your comment? Stating that "in all cases where I ever [used inheritance] or let it go through review I ended up regretting it" is silly. If you mean something else then please explain.

@JeroenDeDauw

Copy link
Copy Markdown
Contributor

Stating that "in all cases where I ever [used inheritance] or let it go through review I ended up regretting it" is silly.

And why do you think this is silly?

You realize that I don't see a way forward in your comment?

Well, you can either get rid of the inheritance or take me up on "If you want I will do a pass on this to get rid of that."

@thiemowmde

Copy link
Copy Markdown
Contributor Author

And why do you think this is silly?

It's called OOP. Get used to it. Is there anything wrong with the actual test? Hard to understand what it does? Anything not covered? Anything tested that should not be part of the contract of the handler? No? Then please don't waste our time. I deny to overengineer such a straightforward thing.

@tobijat

tobijat commented Sep 2, 2014

Copy link
Copy Markdown
Member

I would go for Jeroen's "I will do a pass on this to get rid of that.".
The whole discussion here leads nowhere, so investing some time to have a second solution for comparison might be worth it. Looking at actual implementations is mostly better than just discuss it in theory.
Deal?

@thiemowmde

Copy link
Copy Markdown
Contributor Author

This component still does have zero integration tests. It's more than 4 months now that I wrote them (#112 and #114). And you are asking why queries are delayed? Please, please, for gods sake, decide: What's worse? Having no integrations tests or this? (Whatever is wrong with my coding style, I never got enough information to understand what's so horrifying that it can't be merged.) Pinging @tobijat.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

3 participants