Code coverage #141

bandophahita · 2024-04-05T18:24:40Z

Added tests where coverage was lacking and added configuration to skip portions of files that do not need to be covered.

bandophahita · 2024-04-10T16:37:04Z

I just realized we don't have any coverage for gravitas argument in beat. I'll add that.

perrygoy

Phew i actually had a lot to say on this one!

perrygoy · 2024-04-30T21:35:55Z

screenpy/resolutions/custom_matchers/is_in_bounds.py

+    """
+    Matches a number which is in the given range.


This first line should still be level with the """. Also i don't think we can have the . at the end of the URL like that.

(Well by jove, we can, but it breaks the link-to-the-heading.)

"""Matches a number which is in the given range. Only supports proper intervals. https://en.wikipedia.org/wiki/Interval_%28mathematics%29#Notations_for_intervals """

I think that was done automatically by the pydocstyle rules. I would never have done that myself.
Interestingly enough, when I run the rules now it doesn't seem to add the period. I'm guessing that got fixed.

Sorry i'm coming back to this so late, but if you move the sentence back up to the """s, does linting move it back down again?

Fixed. Latest version of ruff isn't trying to reformat the first line anymore.

screenpy/resolutions/custom_matchers/is_in_bounds.py

perrygoy · 2024-04-30T21:39:10Z

screenpy/resolutions/has_length.py

+    @property
+    def length_to_log(self) -> str:
+        """Represent the length in a log-friendly way."""
+        return represent_prop(self.length)


Make sure to add this method to the exclude-members list fo HasLength in docs/api/resolutions.rst!

To be clear, should we be doing that with all the others as well? None of the resolutions are currently excluding the x_to_log properties. I would imagine we want to be consistent.

Looking at the published docs, I don't see any of the x_to_log properties being displayed for the other resolutions.

Oh, it's because none of the Resolutions have :members: directive, which is obvious to me now because we don't actually have any methods on resolutions (yet).

I see you added those directives, but now i'm wondering—should we add the directives to exclude those members just in case we someday do add the :members: directive, or should we just assume Resolutions will never have methods on them?

I had similar thought, but I wasn't sure. Is that pattern going to fit every Resolution?

I guess we can remove the :exclude-members: directives that you added. I didn't look before i asked for this; having just done a bunch of editing Actions documentation i just assumed we needed it.

I'd rather be consistent for now and deal with that when we need to.

They are removed.

screenpy/resolutions/is_close_to.py

tests/test_pacing.py

perrygoy · 2024-04-30T21:43:04Z

tests/test_settings.py

-    def test__parse_pyproject_toml_file_exists(self) -> None:
-        MockedPath = mock.MagicMock(spec=Path)
+    @mock.patch("screenpy.configuration.Path", autospec=True)
+    def test__parse_pyproject_toml_file_exists(self, MockedPath: mock.Mock) -> None:
        MockedPath.cwd.return_value.__truediv__.return_value = MockedPath
        MockedPath.is_file.return_value = True
        test_data = (
            b"[tool.screenpy]\nTIMEOUT = 500"
            b"\n\n[tool.screenpy.stdoutadapter]\nINDENT_SIZE = 500"
        )
        mock_open = mock.mock_open(read_data=test_data)
+        MockedPath.open.side_effect = mock_open.side_effect
+        MockedPath.open.return_value = mock_open.return_value

-        with mock.patch("pathlib.Path.open", mock_open):
-            pyproject_config = PyprojectTomlConfig(ScreenPySettings)
-            pyproject_config._parse_pyproject_toml()
+        pyproject_config = PyprojectTomlConfig(ScreenPySettings)
+        pyproject_config._parse_pyproject_toml()


Are these changes to limit the scope of the Path that was mocked?

It limits what methods can be called on the mock object. If you recall a while back the conversation about using autospec to narrowly define what methods can be utilized on the mock objects. This would avoid any potential false positives due to a typo.

# mock has no issues with calling methods that don't actually exist # on the original object being mocked obj = mock.Mock obj.mispeeled_method()

The previous code was only and specifically mocking the Path.open method with the specially-crafted mock_open function from unittest.mock. I don't think we would have run into the "mispeeled method" problem here.

It is a very low chance possibility, but wasn't zero. This makes it zero.

I'm still not sure it's necessary:

Python 3.12.3 (main, Apr 9 2024, 08:09:14) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> from unittest import mock >>> mo = mock.mock_open(read_data="") >>> mo.mispeeled_method() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/opt/homebrew/Cellar/[email protected]/3.12.3/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/mock.py", line 658, in __getattr__ raise AttributeError("Mock object has no attribute %r" % name) AttributeError: Mock object has no attribute 'mispeeled_method'

I actually think mocking more of Path than we need to would make the test less valuable, honestly.

If I roll back the mock changes, coverage in configuration.py drops to 98%.
The test__parse_pyproject_toml_file_does_not_exist never seems to hit the following line in PyprojectTomlConfig._parse_pyproject_toml

if not pyproject_path.is_file(): self.toml_config = {}

And that is due to the fact that we weren't patching Path in configuration.py before. With these changes, we get that coverage.

I think that missing that line doesn't point to us needing to mock more, i think it means something about our test just above is incorrect. I don't think mocking further is the right answer, we need to look at why the test just above this comment (test__parse_pyproject_toml_file_does_not_exist) was not hitting this line of code, when it really ought to be.

I wouldn't consider this mocking more, but more along the lines of mocking differently.

In the old version we were creating a mock object but that object was never being utilized at the point in the code where we needed it.

Previously we were only mocking the Path.open and the Path.is_file methods (well, trying to, on that second one). Now we're mocking the entire Path class.

tests/test_speech_tools.py

…any of the test modules.

perrygoy · 2024-07-31T23:40:24Z

docs/api/resolutions.rst

+    :exclude-members: length_to_log, item_plural

 IsCloseTo
 ---------

 **Aliases**: ``CloseTo``

 .. autoclass:: IsCloseTo
+    :exclude-members: delta_to_log, num_to_log


As mentioned in the comments below, you can remove these :exclude-members: directives because we don't have :members: on here to begin with.

Gotcha. Removing them. (I really ought to take some time to learn more about autodoc)

…R comments)

…ise_sources` which causes it to complain about RUF100 (unused noqa) while pylint does not.

MarcelWilson

Looks good

perrygoy

At long last! Sorry for the long delay.

bandophahita requested a review from a team April 5, 2024 18:24

perrygoy reviewed Apr 30, 2024

View reviewed changes

bandophahita added 27 commits May 23, 2024 12:15

adding coverage exclusions

ff92ccc

adding coverage for represent_prop

22dec67

adding coverage for ContainsTheEntry

663f5b5

adding coverage for ContainsItemMatching

eec2d8a

adding coverage for ContainsTheItem

c8fdcae

adding coverage for ContainsTheKey

1782394

adding coverage for ContainsTheText

02d6b2a

adding coverage for ContainsTheValue

751a377

adding coverage for IsEmpty

fb8143d

adding coverage for EndsWith

86a23d4

adding coverage for HasLength

b5f2cd4

adding coverage for IsCloseTo

153ee7d

adding coverage for IsEqualTo

ad35375

adding coverage for IsGreaterThan

7510df4

adding coverage for IsGreaterThanOrEqualTo

6427f65

adding coverage for IsInRange

74b7f55

adding coverage for IsLessThan

fa6cc04

adding coverage for IsLessThanOrEqualTo

8bcc840

adding coverage for IsLessThanOrEqualTo

94ae424

adding coverage for IsNot

d7042b1

adding coverage for ReadsExactly

35bb3a8

adding coverage for StartsWith

d315b65

adding coverage for custom matchers

2d509f9

ensuring proper format of logged pattern

d9fb588

cleanup

4db9282

fixed path mocking to avoid false positive

3bd7978

mah gawd, all da coverage

cd6aac8

bandophahita added 5 commits May 23, 2024 12:16

adjusting logic based on feedback

5ffb6fb

fixing oops caused by pydocstyle in earlier version of ruff.

4b133a0

configuring coverage to skip reporting of tests

179628c

adding exclusion of properties based on feedback

37281dd

removing coverage exclusions by-line since we are no longer covering …

27ba597

…any of the test modules.

bandophahita force-pushed the code_coverage branch from 7b342be to 27ba597 Compare May 23, 2024 17:16

removing redundant coverage exclusion

7fa107f

perrygoy reviewed Jul 31, 2024

View reviewed changes

bandophahita and others added 9 commits August 1, 2024 10:01

removing exclude since we don't have :members: to begin with (re: P…

7c6706c

…R comments)

fixing what earlier version of ruff changed.

115dbea

updating deps and latest version of ruff

9fa7a92

ignoring pycharm project files in git

f23b2c8

helps if you dont have spaces in front

39ac4be

updating from cookiecutter cruft template

64b11b5

latest version of ruff no longer triggers PLR0913 on `settings_custom…

c9e54a5

…ise_sources` which causes it to complain about RUF100 (unused noqa) while pylint does not.

use a dummy settings class instead of mocking.

fae9b8b

update dependencies.

0c6de2c

MarcelWilson approved these changes Aug 2, 2024

View reviewed changes

perrygoy approved these changes Aug 2, 2024

View reviewed changes

bandophahita merged commit b5be493 into ScreenPyHQ:trunk Aug 2, 2024
12 checks passed

bandophahita deleted the code_coverage branch August 2, 2024 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code coverage #141

Code coverage #141

bandophahita commented Apr 5, 2024

bandophahita commented Apr 10, 2024

perrygoy left a comment

perrygoy Apr 30, 2024 •

edited

Loading

bandophahita May 23, 2024

perrygoy Jul 31, 2024

bandophahita Aug 1, 2024

perrygoy Apr 30, 2024

bandophahita May 23, 2024

bandophahita May 23, 2024

perrygoy May 23, 2024

bandophahita May 23, 2024

perrygoy May 23, 2024

bandophahita Aug 1, 2024

perrygoy Apr 30, 2024

bandophahita May 23, 2024 •

edited

Loading

perrygoy May 23, 2024

bandophahita May 23, 2024

perrygoy May 23, 2024

MarcelWilson Jun 5, 2024 •

edited

Loading

perrygoy Jul 31, 2024

bandophahita Aug 1, 2024 •

edited

Loading

perrygoy Aug 1, 2024

perrygoy Jul 31, 2024

bandophahita Aug 1, 2024

MarcelWilson left a comment

perrygoy left a comment

Code coverage #141

Code coverage #141

Conversation

bandophahita commented Apr 5, 2024

bandophahita commented Apr 10, 2024

perrygoy left a comment

Choose a reason for hiding this comment

perrygoy Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bandophahita May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcelWilson Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bandophahita Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcelWilson left a comment

Choose a reason for hiding this comment

perrygoy left a comment

Choose a reason for hiding this comment

perrygoy Apr 30, 2024 •

edited

Loading

bandophahita May 23, 2024 •

edited

Loading

MarcelWilson Jun 5, 2024 •

edited

Loading

bandophahita Aug 1, 2024 •

edited

Loading