#6165 enhancement closed fixed (fixed)

Docstring for twisted.trial.unittest.SynchronousTestCase.mktemp not informative enough

Reported by: meissenPlate Owned by: tom.prince
Priority: low Milestone:
Component: trial Keywords: documentation
Cc: jml, tom.prince@… Branch: branches/mktemp-api-docs-6165
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

twisted.trial.unittest.SynchronousTestCase.mktemp is used to create temporary directories and files in unit tests. This is its docstring:

        """
        Returns a unique name that may be used as either a temporary directory
        or filename.

        @note: you must call os.mkdir on the value returned from this method if
            you wish to use it as a directory!

        @return: C{str}
        """ 

Unique in what context? Could it conflict with a name outside the current working directory? Outside of a standard Twisted distribution? Or is it a magic function that magically gives universally unique names?

Generated Doc.s: http://twistedmatrix.com/documents/current/api/twisted.trial.unittest.SynchronousTestCase.html#mktemp

Source: http://twistedmatrix.com/trac/browser/tags/releases/twisted-12.2.0/twisted/trial/unittest.py#L947

Improved docstring:

        """
        Return a relative path that is guaranteed to be unique within the
        current working directory. Create every directory between the current 
        working directory and the last one if necessary. Do not create the 
        last directory/file.
        
        For a temporary directory call os.mkdir on the path, for a temporary
        file, just create the file (e.g. by opening the path for writing and
        then closing it).

        Trial should delete the temporary file automatically.

        @return: C{str}
        """

Internally it uses tempfile.mkdtemp (from the standard library), in combination with the name of the test module, test suite, and test.

It CREATES a short directory hierarchy like ".\twisted.test.test_compat\ExecfileCompatTestCase\test_execfileGlobals\fqsdxf" and returns a string like ".\twisted.test.test_compat\ExecfileCompatTestCase\test_execfileGlobals\fqsdxf\temp". The random portion is coming from tempfile.mkdtemp.

Attachments (3)

ticket_6165.diff (1.3 KB) - added by meissenPlate 21 months ago.
Improve docstring for twisted.trial._synctest.SynchronousTestCase.mktemp.
ticket_6165.2.diff (1.3 KB) - added by meissenPlate 21 months ago.
Fixed a dumb typo in the first patch's topfile entry.
ticket_6165_3.diff (1.3 KB) - added by meissenPlate 20 months ago.
Discovered misconfigured vimrc: was not auto-deleting trailing whitespace;; fixed whitespace.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 21 months ago by DefaultCC Plugin

  • Cc jml added

Changed 21 months ago by meissenPlate

Improve docstring for twisted.trial._synctest.SynchronousTestCase.mktemp.

Changed 21 months ago by meissenPlate

Fixed a dumb typo in the first patch's topfile entry.

comment:2 Changed 21 months ago by meissenPlate

  • Keywords review added

Changed 20 months ago by meissenPlate

Discovered misconfigured vimrc: was not auto-deleting trailing whitespace;; fixed whitespace.

comment:3 Changed 19 months ago by tom.prince

  • Cc tom.prince@… added

Experimentally, it appears that trial doesn't in fact delete the directory so created.

comment:4 Changed 19 months ago by meissenPlate

You are correct. (Though Trial will delete the files the next time it runs.) Also the directory isn't even created in the CWD. I don't know where I got that. It's created in twisted/_trial_temp.

Maybe the original docstring is best afterall. Just call open or mkdir on the path and run with it, don't worry about where it is or what happens to it. Probably its not worth documenting anything more specific than that.

I'm inclined to close this as wontfix and forget about it. Sorry for the trouble.

comment:5 Changed 19 months ago by tom.prince

Well, it is probably worth documenting that the path is relative to the current directory. And that the path returned is a fresh name in a directory (which is a subdirectory of the current directory).

comment:6 Changed 18 months ago by exarkun

  • Keywords review removed
  • Owner set to exarkun

Indeed. Thanks for working on this. It seems like at least part of this change is desirable. I'll see if I can split that part out.

comment:7 Changed 18 months ago by exarkun

  • Author set to exarkun
  • Branch set to branches/mktemp-api-docs-6165

(In [36876]) Branching to 'mktemp-api-docs-6165'

comment:8 Changed 18 months ago by exarkun

  • Keywords documentation review added
  • Owner exarkun deleted

How about that.

comment:9 Changed 18 months ago by tom.prince

  • Keywords review removed
  • Owner set to tom.prince

This looks good. I'll merge it.

comment:10 Changed 18 months ago by tomprince

  • Resolution set to fixed
  • Status changed from new to closed

(In [37071]) Merge mktemp-api-docs-6165: Improve documentation of twisted.trial.unittest.SynchronousTestCase.mktemp

Author: meissenPlate, exarkun
Reviewers: exarkun, tom.prince
Fixes: #6165

The original documentation didn't give any direction no how it should
be use used. Explain what it returns and how to use it.

Note: See TracTickets for help on using tickets.