Some excellent pytest articles/presentations

[Brian Theado]

I came across these articles about pytest recently and thought they might be of interest to other developers here.

These slides start basic, but cover a lot of ground and I learned a lot:

https://nedbatchelder.com/text/test3.html

Not the main thrust of the presentation, but one quote I liked:

"The kind of change I’m talking about here is refactoring your product code so that your tests can get at the parts they want to. There’s nothing wrong with using your testing as a way to challenge the modularity of your code. Usually, improving the structure to benefit tests also ends up helping you in other ways."

Another quote I found useful. Based on this explanation, I changed some tests I happened to be writing at the time (i.e moving more code out from the test and into fixtures). Having the error vs. failure distinction is useful in troubleshooting failed tests:

"Exceptions in the fixtures are counted as Errors, whereas exceptions in the test function are Failures. This is a subtle but important distinction. A Failure means the test ran, and detected a problem in the product code. An Error means we couldn’t run the test properly.

This distinction is one reason to put set-up code in fixtures, but there are others as we’ll see."

The below two were linked from the above presentation:

https://salmonmode.github.io/2019/03/29/building-good-tests.html 

This article also had some snips related to the quotes I already pasted above (emphasis mine):

"Testing code should not be difficult. If the code is hard to test, then the code is hard to use. This is also indicative of tightly coupled code."

"Steps in the setup process can be broken out into standalone fixtures that should throw errors if something didn’t go well. When those errors are thrown, they will show up as an error in the test report, rather than a failure in the test itself. When a test has an error, it should represent that the desired state for that test could not be reached. If a test fails, it should represent that the state was reached, but something about the state is wrong"

https://github.com/okken/pycascades2020 - Multiply your Testing Effectiveness with Parametrized Testing

I haven't gone through this one as thoroughly, but it also seems pretty good.

Brian

[Edward K. Ream]

On Fri, Mar 13, 2020 at 8:39 PM Brian Theado wrote:

I came across these articles about pytest recently and thought they might be of interest to other developers here.

https://nedbatchelder.com/text/test3.html

Thanks for the link. I'll take a close look.

...one quote I liked:

"The kind of change I’m talking about here is refactoring your product code so that your tests can get at the parts they want to. There’s nothing wrong with using your testing as a way to challenge the modularity of your code. Usually, improving the structure to benefit tests also ends up helping you in other ways."

Even better, in some cases the coverage reports encouraged me to eliminate code completely.

The below two were linked from the above presentation:

https://salmonmode.github.io/2019/03/29/building-good-tests.html 

This article also had some snips related to the quotes I already pasted above (emphasis mine):

"Testing code should not be difficult. If the code is hard to test, then the code is hard to use. This is also indicative of tightly coupled code."

Hmm.

leoAst.py is the kind of code that readily lends itself to unit testing.

Otoh, Leo's Qt dock code depends on complex state set on program startup and shutdown, as well as arbitrary sequences of user actions. I agree that the code is hard to test and hard to use. That does not mean that testing that code will ever be easy. If anyone knows how to test the Qt code, I would appreciate knowing how.

Edward

[Thomas Passin]

On Saturday, March 14, 2020 at 7:33:28 AM UTC-4, Edward K. Ream wrote:

Otoh, Leo's Qt dock code depends on complex state set on program startup and shutdown, as well as arbitrary sequences of user actions. I agree that the code is hard to test and hard to use. That does not mean that testing that code will ever be easy. If anyone knows how to test the Qt code, I would appreciate knowing how.

On my main programming gig, I develop and support an application, similar to but not the same as assigning medical diagnostic codes.  Given a query, the system has to produce X number of candidate codes and score them.  Trouble is, there is no purely objective way to say definitively which responses are "correct".  Some are obviously wrong, but it's for the most part a human judgement call.

Any minor change to the ranking algorithm produces changes in the results.  But I can't automate testing because there is no correct or even standard answer.  The best that I've figured out is to have a person rate the results based on several subjective criteria.  And to be thorough, you need to rate at least 300 query results to get good enough statistics to show an improvement.

[vitalije]

That does not mean that testing that code will ever be easy. If anyone knows how to test the Qt code, I would appreciate knowing how.

Edward

No, it never will be easy, but it can be made easier than it is now. The way to achieve this is straightforward but it requires some effort. The coupled modules should be decoupled as much as possible. From the current state it is not obvious how decoupling can be achieved, but the decoupling process must start somewhere. It can't be started unless you as Leo's creator and main developer do not wish to consider decoupling as a worthy goal. Let me give an example of the unnecessary couplings I have in mind.

About a year ago, when I was experimenting with the history_tracer plugin, I wished to create a small window with the scale and a text widget. Changing the scale widget should show in the text widget the version of the selected node. I wished to have syntax highlighting in the text widget. Now, Leo has several classes related to highlighting the source code, but I could not use any of them. They are all coupled to the commander, so it is not possible to use them for highlighting anything other but the body widget. The idea behind object oriented programing is code reuse. But with the current implementation of syntax highlighters in Leo the code reuse is not possible. I had to implement another highlighter if I wanted to make a qt display widget. In this particular case I have found that it was easier for me to make a web display of the history. Can the PygmentsColorizer or JEditColorizer  be decoupled from the commander? The answer is yes, they can. Both of them currently use commander instance to check the tree in order to find which language should they highlight and to access configuration. Instead it should be the commander's responsibility to configure them and tell them which language should they use. They should not bother themselves with the tree. After all they are highlighting only the text of one widget. If they were implemented that way, it would have been trivial to use them on any text widget. This is just one small example. To fix this some changes are necessary on both sides in the colorizers and in the commander. But those changes will never be made unless you Edward are willing to accept that code can be simplified and that the simplification on its own is a worthy goal. The usual response from you in the past was that the code was sound as it was and that no changes should be made, that the code was inherently complex and that we couldn't do anything about it.

I really believe that doing decoupling piece by piece, module by module, can make Leo's code much easier to test and more importantly easier to (re)use.

Vitalije

[Thomas Passin]

On Saturday, March 14, 2020 at 10:12:06 AM UTC-4, vitalije wrote:

That does not mean that testing that code will ever be easy. If anyone knows how to test the Qt code, I would appreciate knowing how.

[snip]

I really believe that doing decoupling piece by piece, module by module, can make Leo's code much easier to test and more importantly easier to (re)use.

Vitalije

Yes, this is the concept of repaying technical debt.  It can mainly happen by refactoring a piece when that piece comes up for work anyway.  Leo's code base is way too large to do much else, except maybe to have a plan to refactor say one part every minor release.

[Edward K. Ream]

On Sat, Mar 14, 2020 at 9:12 AM vitalije <vita...@gmail.com> wrote:

The way to achieve this is straightforward but it requires some effort. The coupled modules should be decoupled as much as possible. From the current state it is not obvious how decoupling can be achieved, but the decoupling process must start somewhere. It can't be started unless you as Leo's creator and main developer do not wish to consider decoupling as a worthy goal.

Imo, improving Leo's code base is a worthy goal. However, I am not inclined to do it merely to add or improve unit tests.

Let me give an example of the unnecessary couplings I have in mind.

About a year ago, when I was experimenting with the history_tracer plugin, I wished to create a small window with the scale and a text widget. Changing the scale widget should show in the text widget the version of the selected node. I wished to have syntax highlighting in the text widget. Now, Leo has several classes related to highlighting the source code, but I could not use any of them. They are all coupled to the commander, so it is not possible to use them for highlighting anything other but the body widget.

Yes, this is a problem. At present, the commander is needed to gain access to settings.

The idea behind object oriented programing is code reuse. But with the current implementation of syntax highlighters in Leo the code reuse is not possible. I had to implement another highlighter if I wanted to make a qt display widget.

Another idea would be to create a shim commander that only provided the necessary "features" of the real commander.

The g.TracingNullObject class helps create shim objects.  Instead of a valid commander c, use something like:

c = g.TracingNullObject(tag='coloring.c')

This will show you how the colorizer uses c. It's not guaranteed to work without crashing, but it will provide hints about what is needed in, say, a ShimColorizerCommander class.

I really believe that doing decoupling piece by piece, module by module, can make Leo's code much easier to test and more importantly easier to (re)use.

I am inclined to agree with you. However, I am not convinced this is the main line of Leo's development.

Edward

[Thomas Passin]

I agree with Edward.  This is the kind of thing that causes people to want to re-implement, start again to take advantage of what they have learned over the years.  But the history of re-implementations contains a lot of sad failures.  It's just too big a job, the new implementations have their own bugs, and by the time they get done, the original version has developed beyond that the new one has implemented.

This kind of thinking - test-driven, with good separation of concerns, etc. - is good to apply to new development when it can be done and still fit into Leo's existing framework.  That's modulo the difficulty of testing GUI-related code, of course. 

[Edward K. Ream]

On Sun, Mar 15, 2020 at 9:46 AM Thomas Passin <tbp1...@gmail.com> wrote:

> the history of re-implementations contains a lot of sad failures.  It's just too big a job, the new implementations have their own bugs, and by the time they get done, the original version has developed beyond that the new one has implemented.

I have no intention of doing a grand reimplementation. However, many of Vitalije's suggestions have lead to real improvements. I'll look at all specific suggestions.

Edward

[Brian Theado]

Edward,

On Sun, Mar 15, 2020 at 11:38 AM Edward K. Ream <edre...@gmail.com> wrote:

> the history of re-implementations contains a lot of sad failures.  It's just too big a job, the new implementations have their own bugs, and by the time they get done, the original version has developed beyond that the new one has implemented.

I have no intention of doing a grand reimplementation. However, many of Vitalije's suggestions have lead to real improvements.

I'll look at all specific suggestions.

What do you think about the following specific suggestion?

Change all 'g.app' references in the methods of the LeoApp class to 'self'. Or add 'app = self' to the method and change 'g.app' to 'app'.

Example diffs for one method:

@@ -1444,57 +1450,61 @@ class LeoApp:

     @cmd('quit-leo')

     def onQuit(self, event=None):

         """Exit Leo, prompting to save unsaved outlines first."""

-        if 'shutdown' in g.app.debug:

+        app = self

+        if 'shutdown' in app.debug:

             g.trace()

-        g.app.quitting = True

-        if g.app.loaded_session and g.app.sessionManager:

-            g.app.sessionManager.save_snapshot()

-        while g.app.windowList:

-            w = g.app.windowList[0]

-            if not g.app.closeLeoWindow(w):

+        app.quitting = True

+        if app.loaded_session and app.sessionManager:

+            app.sessionManager.save_snapshot()

+        while app.windowList:

+            w = app.windowList[0]

+            if not app.closeLeoWindow(w):

                 break

-        if g.app.windowList:

-            g.app.quitting = False # If we get here the quit has been disabled.

+        if app.windowList:

+            app.quitting = False # If we get here the quit has been disabled.

Brian

[Edward K. Ream]

On Sun, Mar 15, 2020 at 1:29 PM Brian Theado <brian....@gmail.com> wrote:

What do you think about the following specific suggestion?

Change all 'g.app' references in the methods of the LeoApp class to 'self'. Or add 'app = self' to the method and change 'g.app' to 'app'.

Sure, it's possible. This is a pattern I use in most classes. I don't remember if I had a reason for using g.app.

In any case, this change is not going to make much of a difference. I suppose I would accept a PR...

Edward

[Edward K. Ream]

On Sunday, March 15, 2020 at 3:41:09 PM UTC-5, Edward K. Ream wrote:

On Sun, Mar 15, 2020 at 1:29 PM Brian Theado wrote:

What do you think about the following specific suggestion?

Change all 'g.app' references in the methods of the LeoApp class to 'self'. Or add 'app = self' to the method and change 'g.app' to 'app'.

When I awoke this morning I understood why you might suggest this. It would, supposedly, make it possible to instantiate multiple instances of the LeoApp class.  I have just created #1537 for this.

As noted in the issue, it's not clear whether changing `g.app` to `self` in the LeoApp class would make any real difference. It might well mislead people.

The fact that g.app is a singleton instance of the LeoApp class is pretty much baked into Leo. For instance, g.app.db is an external file. Do we really want to complicate the code by creating a dummy instance of g.app.db in some cases?

I am willing to discuss this, but I suspect it complicate Leo's code rather than simplify it.

Edward

[Thomas Passin]

On Monday, March 16, 2020 at 6:23:50 AM UTC-4, Edward K. Ream wrote:

As noted in the issue, it's not clear whether changing `g.app` to `self` in the LeoApp class would make any real difference. It might well mislead people.

I always thought that you had used this kind of idiom as a mnemonic aid.  It has worked that way for me, anyway.

[Edward K. Ream]

Heh, That's probably right. I almost certainly did so without considering that the consequences, namely that the LeoApp instance must be a singleton.

To be clear, imo it's fine with that the LeoApp instance is a singleton. Lastly, trying to allow the LeoApp class not to be a singleton will likely be far more fraught than anyone, including me, might naively expect.

Edward

[Brian Theado]

On Mon, Mar 16, 2020 at 6:23 AM Edward K. Ream <edre...@gmail.com> wrote:

When I awoke this morning I understood why you might suggest this. It would, supposedly, make it possible to instantiate multiple instances of the LeoApp class.  I have just created #1537 for this.

Thanks. Multiple instances of the class is just one possible benefit of reduced coupling. I may make a pull request for this and if I do, I will tag it with #1537.

 

As noted in the issue, it's not clear whether changing `g.app` to `self` in the LeoApp class would make any real difference. It might well mislead people.

I agree it is just a drop in the ocean and to me it still makes a difference. There are always tradeoffs. Is referencing a global variable rather than self in every single method the best (or even a good) way to not mislead people about the (current) requirement that LeoApp be a singleton object? I don't think it is.

 

The fact that g.app is a singleton instance of the LeoApp class is pretty much baked into Leo. For instance, g.app.db is an external file. Do we really want to complicate the code by creating a dummy instance of g.app.db in some cases?

I know there are many other examples of tight coupling throughout leo and you are just picking g.app.db as one, but for that case leobridge already uses g.NullObject for g.app.db, so the rest of leo code must already handle that case.

[Edward K. Ream]

Thanks for these comments. We seem to be on the same page.

Edward