Repository: HypothesisWorks/hypothesis
Pull request: Fix #2106. Defer directory creation to DirectoryBasedExampleDatabase
This PR aims to delay creation of the `.hypothesis/examples` directory until it is actually needed. This is done by shifting responsibility for creation from `configuration.py` to `DirectoryBasedExampleDatabase.save`.

Be sure to let me know if anything else is needed from my side :)
And please point me to another issue I could pick up so I can keep working towards my hacktoberfest t-shirt!

Fixes #2106

Problem statement:
.hypothesis directory always created (pytest plugin)
Hi,

I've just started using hypothesis. Many thanks for a great package. One small issue (which I couldn't find reported elsewhere but maybe my lack of google-fu...) is that, now I have it installed, any time I run pytest (whether or not the particular package I'm testing uses hypothesis) a .hypothesis directory is created. e.g.

Minimal test-case given an environment with pytest and hypothesis installed:
```shell
mkdir t
cd t
echo "def test_me(): assert True, 'what?'\n" > test_me.py
pytest       # 1 test passes
ls -la         # shows a .hypothesis directory (amongst others)
```

It's not harmful, but it was a bit surprising.

Hints from issue discussion:
It would be good if we deferred  creation of that directory to the first write! 
I'd be happy to have a go at this in the spirit of hacktoberfest (so somewhere next week).
As I see it this would require only a few small changes:
1) No longer make the directory as part of configuration (in `configuration.py`). Instead, DirectoryBasedExampleDatabase gets to create it on `__init__` or `save`.
2) Change the permission checks in `_db_for_path`. That would require the assumption that if we can write in the base folder of the __hypothesis_home_dir, we can make our database dir there. Only applicable if the database dir does not exist yet, otherwise we can check the permission of that folder.
3) Tests for the new behaviour.

Would this sum it up, or am I missing something crucial that would result in an awful pile of work and refactoring?
That sounds about right to me - it should indeed be a nicely scoped PR for hacktoberfest :smile: 
Great - thanks for looking into it! (And hope you get the t-shirt ;) ).

Commit message: Address comments, resolve failing CI

Currently hypothesis will make the .hypothesis directory if there are failing tests, or something calls charmap (such as st.text). Otherwise, the directory is not made.

Patch to review:
diff --git a/hypothesis-python/RELEASE.rst b/hypothesis-python/RELEASE.rst
new file mode 100644
index 0000000000..fd35e79fef
--- /dev/null
+++ b/hypothesis-python/RELEASE.rst
@@ -0,0 +1,7 @@
+RELEASE_TYPE: patch
+
+This patch defers creation of the `.hypothesis` directory until we have 
+something to store in it, meaning that it will appear when Hypothesis is
+used rather than simply installed.
+
+Thanks to Peter C Kroon for the Hacktoberfest patch!
\ No newline at end of file
diff --git a/hypothesis-python/src/hypothesis/configuration.py b/hypothesis-python/src/hypothesis/configuration.py
index 42550e622e..cc14894f57 100644
--- a/hypothesis-python/src/hypothesis/configuration.py
+++ b/hypothesis-python/src/hypothesis/configuration.py
@@ -42,7 +42,6 @@ def hypothesis_home_dir():
         __hypothesis_home_directory = os.getenv("HYPOTHESIS_STORAGE_DIRECTORY")
     if not __hypothesis_home_directory:
         __hypothesis_home_directory = __hypothesis_home_directory_default
-        mkdir_p(__hypothesis_home_directory)
     return __hypothesis_home_directory
 
 
diff --git a/hypothesis-python/src/hypothesis/database.py b/hypothesis-python/src/hypothesis/database.py
index 2f38d73189..93fbe15f6c 100644
--- a/hypothesis-python/src/hypothesis/database.py
+++ b/hypothesis-python/src/hypothesis/database.py
@@ -22,12 +22,30 @@
 import warnings
 from hashlib import sha1
 
-from hypothesis.configuration import storage_directory
+from hypothesis.configuration import hypothesis_home_dir
 from hypothesis.errors import HypothesisException, HypothesisWarning
 from hypothesis.internal.compat import hbytes
 from hypothesis.utils.conventions import not_set
 
 
+def _usable_dir(path):
+    """
+    Returns True iff the desired path can be used as database path because
+    either the directory exists and can be used, or its root directory can
+    be used and we can make the directory as needed.
+    """
+    while True:
+        # This loop terminates because the topmost dir ('/' on unix) will always
+        # exist.
+        if os.path.exists(path):
+            usable = os.path.isdir(path) and os.access(
+                path, os.R_OK | os.W_OK | os.X_OK
+            )
+            return usable
+        else:
+            path = os.path.dirname(path)
+
+
 def _db_for_path(path=None):
     if path is not_set:
         if os.getenv("HYPOTHESIS_DATABASE_FILE") is not None:  # pragma: no cover
@@ -36,12 +54,9 @@ def _db_for_path(path=None):
                 "effect.  Configure your database location via a settings profile instead.\n"
                 "https://hypothesis.readthedocs.io/en/latest/settings.html#settings-profiles"
             )
-        # Note: storage_directory attempts to create the dir in question, so
-        # if os.access fails there *must* be a fatal permissions issue.
-        path = storage_directory("examples")
-        if os.access(path, os.R_OK | os.W_OK | os.X_OK):
-            return _db_for_path(path)
-        else:  # pragma: no cover
+
+        path = os.path.join(hypothesis_home_dir(), "examples")
+        if not _usable_dir(path):  # pragma: no cover
             warnings.warn(
                 HypothesisWarning(
                     "The database setting is not configured, and the default "
@@ -158,7 +173,6 @@ def _key_path(self, key):
         except KeyError:
             pass
         directory = os.path.join(self.path, _hash(key))
-        mkdirp(directory)
         self.keypaths[key] = directory
         return directory
 
@@ -167,6 +181,8 @@ def _value_path(self, key, value):
 
     def fetch(self, key):
         kp = self._key_path(key)
+        if not os.path.exists(kp):
+            return
         for path in os.listdir(kp):
             try:
                 with open(os.path.join(kp, path), "rb") as i:
@@ -175,6 +191,10 @@ def fetch(self, key):
                 pass
 
     def save(self, key, value):
+        # Note: we attempt to create the dir in question now. We
+        # already checked for permissions, but there can still be other issues,
+        # e.g. the disk is full
+        mkdirp(self._key_path(key))
         path = self._value_path(key, value)
         if not os.path.exists(path):
             suffix = binascii.hexlify(os.urandom(16))
diff --git a/hypothesis-python/src/hypothesis/internal/charmap.py b/hypothesis-python/src/hypothesis/internal/charmap.py
index a57a44dbd1..9858bff7db 100644
--- a/hypothesis-python/src/hypothesis/internal/charmap.py
+++ b/hypothesis-python/src/hypothesis/internal/charmap.py
@@ -24,7 +24,7 @@
 import tempfile
 import unicodedata
 
-from hypothesis.configuration import storage_directory, tmpdir
+from hypothesis.configuration import hypothesis_home_dir, tmpdir
 from hypothesis.errors import InvalidArgument
 from hypothesis.internal.compat import hunichr
 
@@ -37,7 +37,10 @@
 
 def charmap_file():
     return os.path.join(
-        storage_directory("unicodedata", unicodedata.unidata_version), "charmap.json.gz"
+        hypothesis_home_dir(),
+        "unicodedata",
+        unicodedata.unidata_version,
+        "charmap.json.gz",
     )
 
 
@@ -73,6 +76,8 @@ def charmap():
 
             try:
                 # Write the Unicode table atomically
+                # The call to tmpdir creates the directory (if it doesn't exist
+                # yet). This is fine, since we'll be writing to it anyway.
                 fd, tmpfile = tempfile.mkstemp(dir=tmpdir())
                 os.close(fd)
                 # Explicitly set the mtime to get reproducible output
@@ -80,7 +85,7 @@ def charmap():
                     result = json.dumps(sorted(tmp_charmap.items()))
                     o.write(result.encode())
 
-                os.rename(tmpfile, f)
+                os.renames(tmpfile, f)
             except Exception:
                 pass
 
diff --git a/hypothesis-python/src/hypothesis/internal/reflection.py b/hypothesis-python/src/hypothesis/internal/reflection.py
index 3a157a010b..dc863af5b5 100644
--- a/hypothesis-python/src/hypothesis/internal/reflection.py
+++ b/hypothesis-python/src/hypothesis/internal/reflection.py
@@ -24,6 +24,7 @@
 import ast
 import hashlib
 import inspect
+import os.path
 import re
 import tokenize
 import types
@@ -31,7 +32,7 @@
 from functools import wraps
 from types import ModuleType
 
-from hypothesis.configuration import storage_directory
+from hypothesis.configuration import hypothesis_home_dir
 from hypothesis.internal.compat import (
     ARG_NAME_ATTRIBUTE,
     getfullargspec,
@@ -474,7 +475,7 @@ def check_valid_identifier(identifier):
 
 
 def eval_directory():
-    return storage_directory("eval_source")
+    return os.path.join(hypothesis_home_dir(), "eval_source")
 
 
 eval_cache = {}  # type: dict
diff --git a/hypothesis-python/tests/nocover/test_database_usage.py b/hypothesis-python/tests/nocover/test_database_usage.py
index 6c14051c01..7ca7a2e10e 100644
--- a/hypothesis-python/tests/nocover/test_database_usage.py
+++ b/hypothesis-python/tests/nocover/test_database_usage.py
@@ -17,9 +17,11 @@
 
 from __future__ import absolute_import, division, print_function
 
+import os.path
+
 import hypothesis.strategies as st
 from hypothesis import assume, core, find, given, settings
-from hypothesis.database import InMemoryExampleDatabase
+from hypothesis.database import ExampleDatabase, InMemoryExampleDatabase
 from hypothesis.errors import NoSuchExample, Unsatisfiable
 from hypothesis.internal.compat import hbytes
 from tests.common.utils import (
@@ -145,3 +147,15 @@ def test(i):
         pass
 
     test()
+
+
+@given(st.binary(), st.binary())
+def test_database_not_created_when_not_used(tmp_path_factory, key, value):
+    path = tmp_path_factory.mktemp("hypothesis") / "examples"
+    assert not os.path.exists(str(path))
+    database = ExampleDatabase(path)
+    assert not list(database.fetch(key))
+    assert not os.path.exists(str(path))
+    database.save(key, value)
+    assert os.path.exists(str(path))
+    assert list(database.fetch(key)) == [value]


Code review comments:
diff_hunk: @@ -22,12 +22,30 @@
 import warnings
 from hashlib import sha1
 
-from hypothesis.configuration import storage_directory
+from hypothesis.configuration import hypothesis_home_dir
 from hypothesis.errors import HypothesisException, HypothesisWarning
 from hypothesis.internal.compat import hbytes
 from hypothesis.utils.conventions import not_set
 
 
+def _usable_dir(path):
+    """
+    Returns True iff the desired path can be used as database path because
+    either the directory exists and can be used, or its root directory can
+    be used and we can make the directory as needed.
+    """
+    while True:
+        # This loop terminates because the topmost dir ('/' on unix) will always
+        # exist.
+        if os.path.exists(path):
+            usable = os.path.isdir(path) and os.access(
+                path, os.R_OK | os.W_OK | os.X_OK
+            )
+            return usable
+        else:
+            path = os.path.dirname(path)
path: hypothesis-python/src/hypothesis/database.py
text: @user1:
A slightly shorter version:

```python
while not os.path.exists(path):
    # Loop terminates because the root dir ("/" on unix) always exists
    path = os.path.dirname(path)
return os.path.isdir(path) and os.access(path, os.R_OK | os.W_OK | os.X_OK)
```

diff_hunk: @@ -474,7 +475,7 @@ def check_valid_identifier(identifier):
 
 
 def eval_directory():
-    return storage_directory("eval_source")
+    return os.path.join(hypothesis_home_dir(), "eval_source")
path: hypothesis-python/src/hypothesis/internal/reflection.py
text: @user1:
It appears that this function is tested but never actually used - let's just delete it entirely!

@author:
I noticed, but I was hesitant to remove it. I'll get right on it.

diff_hunk: @@ -22,12 +22,30 @@
 import warnings
 from hashlib import sha1
 
-from hypothesis.configuration import storage_directory
+from hypothesis.configuration import hypothesis_home_dir
path: hypothesis-python/src/hypothesis/database.py
text: @user1:
Having grepped through uses of `storage_directory()`, it doesn't look like there are *any* cases where we want the directory created there...  and it's the only use of `hypothesis_home_dir()`.  Let's inline that logic and delete the single-use `tmpdir()` helper for simplicity.

Then all our calls can just be `storage_directory("some", "paths")` and we don't need to wrap it in `os.path.join()`.