7154-Improve-testdir-documentation-on-makefiles #7239

symonk · 2020-05-22T14:29:58Z

Good afternoon, hope you are in good health in the current climate. I have made some minor adjustments to TestDir and some of its accompanying documentation, thanks for your time and look forward to receiving some feedback. Helps with #7154

Added the ability to Testdir to track created files and allow tests to access this data, includes test coverage for this feature.
Added some documentation around makepyfile/maketxtfile to explain that when a name is not passed, the files contents will be overwritten in place
Include documentation when adding new features.
Include new tests or update existing tests when applicable.
Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.

…ting the same file can occur

symonk · 2020-05-22T14:43:42Z

forgot py35 support, will address

nicoddemus

Thanks a lot for taking the time to contribute this @symonk!

The updated docs are definitely a plus, I'm a bit curious about the use case for Testdir.created_files though. Can you give some examples in tests where this can be useful?

nicoddemus · 2020-05-22T17:39:34Z

src/_pytest/pytester.py

+                testdir.makepyfile("foobar")
+                # to create multiple files, pass kwargs accordingly
+                testdir.makepyfile(custom="foobar")
+                # outcome is test_something.py & custom.py


Suggested change

# outcome is test_something.py & custom.py

# at this point, both 'test_something.py' & 'custom.py' exist in the test directory

nicoddemus · 2020-05-22T17:41:54Z

changelog/7154.improvement.rst

@@ -0,0 +1 @@
+Testdir now keeps track of created file paths, updated Testdir documentation to reflect overwritten files


Suggested change

Testdir now keeps track of created file paths, updated Testdir documentation to reflect overwritten files

``Testdir.created_files`` returns a ``Set`` of paths created by ``make*file`` family of methods.

But I'm curious, what's the use case for this new property?

nicoddemus · 2020-05-22T17:42:10Z

src/_pytest/pytester.py

        return ret

+    @property
+    def created_files(self) -> Set:


Suggested change

def created_files(self) -> Set:

def created_files(self) -> Set[py.path.local]:

nicoddemus · 2020-05-22T17:43:02Z

src/_pytest/pytester.py

@@ -650,8 +652,17 @@ def to_text(s):
            p.write(source.strip().encode(encoding), "wb")
            if ret is None:
                ret = p
+                if ret.strpath not in self._created_files:


I suggest to replace both lines by:

self._created_files.add(ret)

No need to check for membership, and I think keeping a set of py.path.local is better. 👍

nicoddemus · 2020-05-22T17:43:35Z

src/_pytest/pytester.py

+                testdir.maketxtfile("foobar")
+                # to create multiple files, pass kwargs accordingly
+                testdir.maketxtfile(custom="foobar")
+                # outcome is test_something.txt & custom.txt


Suggested change

# outcome is test_something.txt & custom.txt

# at this point, both 'test_something.txt' & 'custom.txt' exist in the test directory

nicoddemus · 2020-05-22T17:45:59Z

src/_pytest/pytester.py

-        """Shortcut for .makefile() with a .py extension."""
+        r"""Shortcut for .makefile() with a .py extension
+        defaults to the test name .py e.g test_foobar.py
+        Calling without kwargs will result in overwriting the initial file contents


I think we can remove this phrase, because it conveys the impression that by calling with kwargs initial file contents are somehow not overwritten.

nicoddemus · 2020-05-22T17:46:19Z

src/_pytest/pytester.py

-        """Shortcut for .makefile() with a .txt extension."""
+        r"""Shortcut for .makefile() with a .txt extension
+        defaults to the test name .txt e.g test_foobar.txt
+        Calling without kwargs will result in overwriting the initial file contents


I think we can remove this phrase, because it conveys the impression that by calling with kwargs initial file contents are somehow not overwritten.

nicoddemus · 2020-05-22T17:48:34Z

testing/test_pytester.py

+def test_testdir_duplicate_paths(testdir):
+    testdir.makepyfile("foo")
+    testdir.makepyfile("bar")
+    assert len(testdir.created_files) == 1


Suggested change

assert len(testdir.created_files) == 1

assert testdir.created_files == {testdir.tmpdir.join("test_testdir_duplicate_paths.py")}

This is better because we are more explicit about the outcome, helping to figure out any problems in case the test fails.

nicoddemus · 2020-05-22T17:48:56Z

testing/test_pytester.py

+def test_files_by_name_are_correct(testdir):
+    testdir.maketxtfile("foo")
+    testdir.maketxtfile(custom="foobar")
+    assert len(testdir.created_files) == 2


Suggested change

assert len(testdir.created_files) == 2

assert testdir.created_files == {testdir.tmpdir.join("test_files_by_name_are_correct.py"), testdir.tmpdir.join("custom.py")}

Ditto.

nicoddemus · 2020-05-22T17:49:35Z

testing/test_pytester.py

+    two = testdir.maketxtfile(data)
+    three = testdir.makeconftest(data)
+    four = testdir.makeini(data)
+    assert {one, two, three, four} == testdir.created_files


Suggested change

assert {one, two, three, four} == testdir.created_files

assert testdir.created_files == {one, two, three, four}

For consistency with the rest of the tests: "obtained == expected"

symonk · 2020-05-22T18:36:12Z

@nicoddemus thanks for the fast feedback, appreciate it - hope you are well. The new property i guess i jumped the gun a bit there, was doing some plugin development and was creating a lot of files as part of my testing, found it easier to use that rather than keep going to the temp directory and inspecting them on FS - updated accordingly the documentation and plucked out the work that isnt really beneficial.

Thanks again

nicoddemus

Thanks a lot @symonk, we appreciate it!

symonk added 2 commits May 22, 2020 15:23

add file tracking to TestDir, update documentation to reflect overwri…

c8bcc49

…ting the same file can occur

fix documentation typos

ad2eb1d

symonk added 2 commits May 22, 2020 16:01

Remove type hint on self attr

1512c96

Fix up docstrings; improve tests

730bab8

nicoddemus requested changes May 22, 2020

View reviewed changes

revert new property, tidy up documentation of testdir functions

ad9ca27

address final documentation changes

19915b6

symonk changed the title ~~7154-Testdir-created-files-and-documentation-improvements~~ 7154-Improve-testdir-documentation-on-makefiles May 22, 2020

symonk requested a review from nicoddemus May 22, 2020 18:56

nicoddemus approved these changes May 23, 2020

View reviewed changes

nicoddemus merged commit 05c22ff into pytest-dev:master May 23, 2020

symonk deleted the 7154-track-testdir-files-and-update-documentation branch May 23, 2020 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

7154-Improve-testdir-documentation-on-makefiles #7239

7154-Improve-testdir-documentation-on-makefiles #7239

Uh oh!

symonk commented May 22, 2020 •

edited

Loading

Uh oh!

symonk commented May 22, 2020

Uh oh!

nicoddemus left a comment

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

nicoddemus May 22, 2020

Uh oh!

symonk commented May 22, 2020

Uh oh!

nicoddemus left a comment

Uh oh!

Uh oh!

	# outcome is test_something.py & custom.py
	# at this point, both 'test_something.py' & 'custom.py' exist in the test directory

		@@ -0,0 +1 @@
		Testdir now keeps track of created file paths, updated Testdir documentation to reflect overwritten files

	Testdir now keeps track of created file paths, updated Testdir documentation to reflect overwritten files
	``Testdir.created_files`` returns a ``Set`` of paths created by ``make*file`` family of methods.

	def created_files(self) -> Set:
	def created_files(self) -> Set[py.path.local]:

	# outcome is test_something.txt & custom.txt
	# at this point, both 'test_something.txt' & 'custom.txt' exist in the test directory

	assert len(testdir.created_files) == 1
	assert testdir.created_files == {testdir.tmpdir.join("test_testdir_duplicate_paths.py")}

	assert len(testdir.created_files) == 2
	assert testdir.created_files == {testdir.tmpdir.join("test_files_by_name_are_correct.py"), testdir.tmpdir.join("custom.py")}

	assert {one, two, three, four} == testdir.created_files
	assert testdir.created_files == {one, two, three, four}

Uh oh!

7154-Improve-testdir-documentation-on-makefiles #7239

7154-Improve-testdir-documentation-on-makefiles #7239

Uh oh!

Conversation

symonk commented May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

symonk commented May 22, 2020

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

symonk commented May 22, 2020

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

symonk commented May 22, 2020 •

edited

Loading