miyuchina / mistletoe

A fast, extensible and spec-compliant Markdown parser in pure Python.
MIT License
811 stars 113 forks source link

Bug fix for #142: pytest failing. #153

Closed anderskaplan closed 2 years ago

anderskaplan commented 2 years ago

Hi, here's a fix for issue #142. The problem was that the GithubWiki token didn't get added to span_token._token_types as intended under pytest.

pbodnar commented 2 years ago

@anderskaplan, thanks for the fix, I will hopefully check it out soon.

As you haven't quite explained what was the culprit, I'm curious if your removal of import of span_token._token_types plays a role in the fix, or not...

anderskaplan commented 2 years ago

It does. I'm not quite sure what the cause of the problem is, but I'd guess it has something to do with how modules are loaded and module-global data is initialized when running within pytest.

So the solution was to reference _token_types with module specifier in the unit test. And then there was no need for the import.

pbodnar commented 2 years ago

@anderskaplan, thanks for the explanation. 👍 I've just checked myself and have come to the same conclusion, i. e. something really weird is going on with the (not-)imported global variable (my first really big "what the heck" thing since I started "playing with Python" a few years ago, I guess ;)).

So the second change of removing autospec argument from the call to mock.Mock() is apparently just a cosmetic one - it looks like the argument was useless.

So I will leave this PR open for a day or two for the case you would like to put the explanation right into the commit message (which I believe is a best practice) before I merge it... :)

anderskaplan commented 2 years ago

Ok, updated the commit message as requested.

And you're right, the Mock autospec argument isn't even implemented for the Mock constructor, so removing it was a cosmetic thing.

pbodnar commented 2 years ago

@anderskaplan, I'm sorry for reopening the topic, but based on the previous comments, I thought rather something like this:

Bug fix for #142: pytest failing.

The problem was that when run under pytest, the test referenced a wrong instance of `_token_types`,
therefore setting mocked RawText to a wrong list.
Solved by referencing `_token_types` with module specifier in the unit test.

So can you please amend the commit once again? I promise it will be for the last time. ;)

(Note that self.assertIsInstance(token, GithubWiki) was passing, so it wasn't about the GithubWiki token really.)

anderskaplan commented 2 years ago

No problem. I've updated the commit message.

pbodnar commented 2 years ago

Thank you. :)