-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Why use object instead of class? #26
Comments
What does a class allow you to do that an object doesn't? I find An If you had an issue with And note that with Minitest is used by Monix, which has thousands of tests running for both JS and the JVM, it's one of the better tested libraries in the ecosystem and we never had any problems with |
would become
and now I can run swoval, which implements file system monitoring in sbt 1.3.0, uses utest which also uses object. It has roughly 250 tests that are nearly all asynchronous and run on jvm and js. I am not saying that object is a horrible design choice or that it can't be worked around. I just think that class is more flexible. Given that this is a strawman, I figured I'd voice that opinion. |
@eatkins in your sample you're dealing with mutable state anyway, the object is just more honest about it. Also I think mutable state shared between multiple tests is absolutely terrible. In Minitest I didn't even want to add setupSuite and teardownSuite. This happened due to Akka and Akka only, because creating that actor system is super expensive. But this does bring back memories of me testing actors. Because the actor system is shared, you can end up polluting its internal queues and so failure in a test can also make other tests fail and then you have to chase down the test that actually failed and that brought down the rest. This is why I think that:
Your opinions are always welcome btw and this is no longer my project so it's not even up to me, suggestions welcome 👍 |
fwiw, I switched from JUnit to scala-verify, and one of my test cases that used to run in a fraction of a second started taking two minutes, I believe because of the usual JVM pitfall of code in the constructor not getting JITted. arguably that was my fault for writing code that was arguably a bit trashy? but it's a really easy pit to fall into (in Scala even more so than Java) |
Another obvious problem with using objects for tests is that it makes it possible for a test to inadvertently leak a resources when a different test accesses a static field or method even if the the resource is correctly cleaned up by the containing test: object FooTest {
val x = 1
val y = new ThreadBackedResourceImplementingAutoCloseable
...
override def tearDownSuite(): Unit = {
y.close()
}
}
object BarTest {
val z = FooTest.x
} Now if I run |
I guess I would support adding support for classes, given the pitfalls expressed above. But I would still like objects to be supported and even be the default in the docs. |
Forgive my ignorance, but I genuinely don't understand the preference for object. I think I understand @alexandru's argument above, but I find it unpersuasive. From my perspective, tests inherently have a dynamic lifecycle that is managed by the test framework so class is the appropriate abstraction. When I see the word object, I see global state and the potential for resource leaks which are precisely what we want to discourage. Really I am asking what real world problem is solved by using object that cannot be solved at the framework level using class? There are problems with object that can be fixed by switching to class but I'm failing to see what problems with class can't be addressed by the test framework. Even with objects the framework still has to manage lifecycle, see e.g. Having classes and objects as valid test implementations seems problematic both because it introduces two ways to do the same thing which is bound to cause downstream arguments and because the name becomes ambiguous. Consider the following: object FooTest extends Test {
...
}
class FooTest extends Test {
...
} In sbt, how would I run one but not the other? Would I have to run |
It isn't about shared mutable state, it's about resources. In an ideal world every single test case would have its own resources. Unfortunately, that can kill performance. In many cases, whether we're talking about akka actor systems, test databases, caches, webservers, etc., the majority of the test runtime is spent creating resources. To make it concrete, suppose I want to test read only database queries. One way to do this is to fill a test database with mock data before all of the test case and drop the data after they complete. At my last job, filling a test postgres server with data could easily take 500ms to 1s while each query against that data might take 1-50ms. As a user, I don't want to be told that my test suite with 10 cases has to take 10 seconds instead of 1 second because I'm doing it wrong by sharing data between cases. When we wanted to test updates, we had to reset the data between test cases which made the tests dramatically slower. While some might argue that these are integration tests rather than unit tests, I personally do not care about the semantic difference. The beauty of this technique is that, when using dependency injection, you don't have to mock your code, you only have to mock your data. Scalatest adapted to this style and we were able to get unit test like feedback while exercising code paths that very closely mimicked production code paths. |
agree.
agree |
I see no state, or at least no mutable state and/or side-effecting state initialisation. But I agree the framework should continue to support the lifecycle hooks. |
To clarify, what I meant is that I ask myself, "what could go wrong with this abstraction?" I think object is a valuable concept in scala and overall prefer it to java style static fields and methods but it does have some pitfalls. |
Yeah, I've gained a newfound respect for Java's lack of
Agreed. "convenient but with pitfalls", very Scala-esque.
Or maybe |
Is either Maybe picking |
That example with the resource leak can very well happen with classes as well: class FooTest {
val x = 1
val y = new InputStream(...)
...
override def tearDownSuite(): Unit = {
y.close()
}
}
class BarTest {
val z = new FooTest().x
} This is still a leak and the garbage collector will not help. I do wonder if we can support both classes and objects. |
Introducing two ways to do the same thing isn't necessarily bad, we aren't dogmatic about it like in Python. Also if something hurts, like in the case of defining both a class and an object with the same name, then you stop doing it. |
Performance issues might be a valid reason for having classes. |
@alexandru sure, you can create the same type of leak using class but it is far less likely. In particular, implicit classes must be defined in an object, not a class, so what my example might actually look like in practice is: object InetAddressTest extends Test {
implicit class InetStringOps(private val s: String) extends AnyVal {
def toInetAddress: java.net.InetAddress = new java.net.InetAddress(s)
}
val y = new TheThingThatWillLeakIfNotClosed
...
} Bar.scala: import InetAddressTest._
object BarTest extends Test {
val blah = "127.0.0.1".toInetAddress
...
} This example is much more difficult to write if tests must be defined in a class because you wouldn't even be able to define the implicit class inside of the test implementation. A counter to this is: "well you should put the implicit class definition in a different object than the test object," but this seems exactly like the kind of thing that companion objects were designed for. And besides, to me this isn't so much about right and wrong but what kind of mistakes are users likely to make and how can we help them avoid them? As an sbt maintainer, I am very sensitive to resource leaks. It is easy to write application code that leaks resources but never causes any problems because it is expected that the resource lives for the duration of the program. In long running sbt sessions, those same resource leaks cause various problems which will generally manifest as either sbt running out of memory or performance degrading so badly that sbt must be restarted (which is frustrating given how long sbt can take to start up). For me, the use of object makes it more likely that our downstream users run into problems. Of course there is no way to prevent users from writing resource leaks in general but I will continue to push for the patterns that reduce their likelihood. |
I agree that it isn't necessarily bad but that doesn't mean it shouldn't be avoided if possible. |
Yes, but at least we can assume that if class is used then each test implementation will be initialized in the same place in the runner code. With objects, it is pretty much impossible to say when any particular test implementation is initialized since it depends on whether and how dependent test implementations access each other's object fields and/or methods. |
How often have you seen test fields being reused in other tests, thus creating leaks? Anecdotally speaking, I can't think of any one such instance. I've never seen it in any of the projects I participated it. Of course we might not be the target demographic and maybe this is a consequence of using actual FP lately and describing resources via Resource. And I'm aware that people find creative ways to shoot themselves in the foot, but I don't find this as being a compelling argument against objects, mostly because you're arguing against simple functions. Personally I like simple functions and I'm pretty much anti-classes at this point 80% of the time 🙂 Most leaks are file handle leaks. And in the case for tests, no matter the type of the leak, mutable resources being reused means that a test can pollute other tests and that's always bad. To me this is super obvious and even though I'm not in the target demographic ... I don't see a good reason to make resource handling safer with a construct that in fact fails to do that and fails hard. Putting things in a But personally I can be persuaded by anecdotes, so do you have any such stories? Or in other worlds, why in the world would somebody define extension methods in the object of a test? Or reusable resources for that matter that need to be imported in other tests? These sound like stories meant for the daily wtf.
😛 |
Also anecdotally speaking — working with |
Cos extension methods read nicer than functions. |
@alexandru I have made my points as clearly as I intend to. I have been writing scala for about 8 years and have seen resource leaks in all forms. I am 100% sure that I have seen some form of the examples I have provided show up in the real world but I'm sorry that I can't recall every specific bug that I have seen over the years. I have no illusions that my experience is comprehensive but I have provided numerous examples in this thread and you keep coming back with the no true scotsman argument. It is exhausting and I'm not particularly inclined to keep debating the validity of my concerns.
This is an opinion presented as fact.
When have I ever argued against simple functions?
This is an opinion that you are projecting onto the world at large. I think a lot of, if not most, people don't think much at all about what they should or shouldn't do in an object or class constructor.
Because people don't design things up front. They add an extension method for one test and then later they write another test and think, "hey, I added an extension method in Foo, let me just use it in Bar!" You may think that's dumb, but that's how code often gets written.
I guess java never should have introduced the All that I have heard from your comments in this thread is that you have a strong opinion about how scala code should be written and the library is designed to facilitate coding in your preferred style. That is fine for Minitest but this is a library that aspires to be a de-facto standard which means people with all kind of styles will use it. |
Btw, with the new lambda encoding it's much easier than before to shoot yourself in the foot with objects, see this REPL bug which I've been working on recently: scala/bug#9076. It's because object initialisation is bound to class initialisation, which blocks all calls from other threads, so you can deadlock yourself if your object initialisation accesses the object from another thread. Here's a nice and terse example: // This works fine:
val x = Future(42)
Await.result(x, 1.second)
// This throws a TimeoutException:
Await.result(Future(42), 1.second) |
In this case should we change: object Main extends App { ... } by: class Main extends App { ... } ? To me a test is just another entrypoint like a main. |
On 2/13/2020 7:04 PM, Dale Wijnand wrote:
It's because object initialisation is bound to class initialisation, which blocks all calls from other threads, so you can deadlock yourself if your object initialisation accesses the object from another thread.
I'm not exactly sure what the Scala compiler does, but in general if
`this` escapes the current thread during the execution of the
constructor, bad things can happen. But this is relevant for classes
too, being a JMM gotcha related to classes.
I know that the code you posted misbehaves in the REPL, it's an old
problem visible when using `Await.result` on a `Future(...)`. I don't
see the relevance though, unless I'm missing something.
I admit that I haven't looked carefully at what causes that particular
problem in the REPL, but if we are talking about the poor man's way of
avoiding cyclic dependencies (also see
https://github.com/lihaoyi/acyclic), then this isn't
relevant because we are arguing against usage of plain functions.
|
I do value your opinions and for whatever it is worth, will take them under consideration.
Note my use of the word "anecdotally". I'm very much aware that we're talking of opinions here - in general software development isn't engineering and computer science isn't an actual science, as I like pointing out the field isn't evolved via the scientific method and in general, if we are not talking about math, then we are talking about opinions and anecdotes. That goes without saying. And compared with others however, I am very careful in underlying my anecdotes and admitting my opinions. I don't like your tone though, even if I'm aware this is the norm in many communities (Reddit and elsewhere) and this discussion is no longer productive, so we'll agree to disagree. Cheers, |
I guess, if you will, it's about "degrees" of "badness", maybe? If I'm guessing correctly,
Do you mean old problem as in old problem in the REPL, or in general? The problem I'm thinking about became more of a problem ever since the Java 8 / Scala 2.12 lambda encoding. It's relevant here because it triggers in the REPL because the REPL (currently, we're working on trying to change it) uses object wrappers as in its implementation (there's no real evaluator - the E in REPL is a lie :o).
In my eyes I see it as arguing for what encoding is better (and, later, if both encodings should be supported). As I stated in the beginning, I prefer object. So, at minimum, I'd like objects supported. I'm happy to give up the docs promoting objects, as I'm becoming more and more convinced that it's a footgun for beginners. If such beginners then want to choose the more strict FP style of Scala, then they'll have to learn about the pitfalls of side-effecting and so forth, but then rejoice in being able to use simple functions in an object. But perhaps when they're still beginners they shouldn't be tested on this while setting up their... tests. 😄 |
@dwijnand I'm coming around to the opinion that Not sure how the implementation would work. One thing I really dislike about |
@alexandru I regret that the tone of this discussion has indeed devolved. I try to keep things civil and respectful and am personally dismayed by much of the discussion that exists around scala on the internet. That being said, I'd challenge you to reflect on how you have engaged with me in this thread. From your first response to my initial comment, you wrote:
This bothered me because it reads to me like you're accusing me of incompetence whether or not that was your intent. Similar to your claim that I "was arguing against simple functions," it felt like you were putting words in my mouth forcing me to defend a claim that I never made. Moreover, you made a number of strong judgments on various stylistic choices, e.g. "why in the world would somebody define extension methods in the object of a test?" that I find completely reasonable. I made a choice to respond the way that I did that was a direct reaction to the emotional content of your previous comments. It admittedly wasn't my best look but it didn't happen in a vacuum. At any rate, I no longer wish to participate in this discussion so, as you said, we'll have to agree to disagree. I am closing this issue because I no longer want to receive notifications. |
I am saddened by this communication breakdown since I have lots of respect for both @eatkins and @alexandru, and I feel like both have tried to make sincere cases. Maybe we can cool off for a few days, and someone else can open a new issue with pros/cons summary. |
I apologize if my communication style put you off. I have strong opinions that I should probably tone down. Rest assured that it wasn't my intention to accuse anyone going against my opinions of incompetence, nor do I have such thoughts. At times I'm also half joking and forget to add the smileys, as if my interlocutor could see the body language. Online communications are hard ¯_(ツ)_/¯ Your input is very valuable and I think the folks here will seriously think of supporting or switching to |
@alexandru water under the bridge. I apologize for overreacting. |
I know that this is forked from minitest, which uses object for test suites, but I somewhat prefer classes ala ScalaTest or Specs2. I'm struggling to remember why, but I remember having an issue in swoval because utest used objects for test suites instead of classes. It doesn't matter if you're just using a build tool test runner, but I think classes are more flexible in general.
The text was updated successfully, but these errors were encountered: