Add a "shared cache" directory

[DiddiLeija]

Closes #72. Add an {envdir}/.shared directory (like @dhermes suggested), and create an API for accesing to it.
I'm a bit new to the concept, so tell me if this is what you want :)

CC @theacodes @cjolowicz

[DiddiLeija]

Ok. At this point, I have nox.session.get_shared_cache_dir, but I don't know at all where should I use it across the sessions.

[DiddiLeija]

Any suggestions?

[theacodes]

Looks like a great start. Some suggestions:

• return a pathlib.Path object instead of a string.
• make it a @property and just call it cache_dir.

[DiddiLeija]

@theacodes I made your suggestions. Now, I don't know if I have to do something to enable the access to cache_dir.

[DiddiLeija]

Why are the build steps on the CI failing?

[DiddiLeija]

Any suggestions here?

[dhermes]

@DiddiLeija You are failing test coverage for the newly added code:

nox/sessions.py         269      3     86      0    99%   191-193

Just need to add tests and cover the new source lines added

[theacodes]

(It's currently a busy time for me as I'm putting together 500 synth modules to ship out, so I'll get to this soon)

[DiddiLeija]

No problem, I understand how does it feels 😄. Review it whenever you want.

[FollowTheProcess]

Hi @DiddiLeija thanks for the contribution. You're pretty close!

I've checked out your PR locally and have played around with it.

runner.envdir is None in this test because it hasn't been defined for the test. Notice how the tests around it explicitly set their runner.envdir like this example below:

def test_create_tmp_twice(self):
    session, runner = self.make_session_and_runner()
    with tempfile.TemporaryDirectory() as root:
        runner.global_config.envdir = root # Here we explicitly set the envdir
        runner.venv.bin = bin
        session.create_tmp()
        tmpdir = session.create_tmp()
        assert session.env["TMPDIR"] == tmpdir
        assert tmpdir.startswith(root)

You should add the following line at the start of test_properties:

def test_properties(self):
    session, runner = self.make_session_and_runner()
    runner.global_config.envdir = ".test" # Set the envdir explicitly, the name doesn't matter

    ...
    # code below

I would also make one other change:

Because session.cache_dir returns a pathlib.Path, this is what we should be testing for in the assertions. So in place of:

# code above
...
assert session.cache_dir == os.path.join(runner.envdir, ".shared")

Instead use:

# code above
...
assert session.cache_dir == pathlib.Path(runner.envdir).joinpath(".shared")

With these changes, the tests should be passing.

[DiddiLeija]

Thanks @FollowTheProcess. I will follow your suggestions.

[DiddiLeija]

I hope this is going to work.

[DiddiLeija]

I think this PR is ready (at least, there aren't great changes to do).

[DiddiLeija]

Hmm... it seems like the tests are looking for .test/test/.shared. But I think it should be .test/.shared, right?

[FollowTheProcess]

Hmm... it seems like the tests are looking for .test/test/.shared. But I think it should be .test/.shared, right?

You're right, my bad for not spotting this, it actually took me a while to figure out myself.

The sessions envdir is cleaned up and joined with the session name by default as all other sessions will want to access their individual envdirs e.g. .nox/tests or .nox/lint. Because what we want here is access to the root .nox directory in order to create .nox/.shared we have to make a few changes to the cache_dir property and it's tests to allow this (see my review comments).

[DiddiLeija]

@FollowTheProcess Done.

[FollowTheProcess]

@FollowTheProcess Done.

Cool, looks like you unindented the test_properties function when you did this, so I just fixed that for you and it now looks like this is passing so congrats!

Remember you should always run tests and formatting etc locally first before pushing up 👍🏻 running nox would have found this (that's how I found it and why nox is great).

[FollowTheProcess]

Just want to get another maintainer to take a look at this and make sure they're happy before merging in. cc @theacodes @dhermes

[cjolowicz]

LGTM

[cjolowicz]

Should the directory be named .cache for consistency with session.cache_dir?

It's a cheap change now, and it would remove a mental translation step (cache_dir -> .shared) for everybody using this in the future.

[FollowTheProcess]

Should the directory be named .cache for consistency with session.cache_dir?

It's a cheap change now, and it would remove a mental translation step (cache_dir -> .shared) for everybody using this in the future.

Yep, makes total sense to me. Agree with your suggestions in the review too.

@DiddiLeija do you want to implement these tweaks or are you happy for us to just fix this up real quick?

[DiddiLeija]

Of course I can do this simple change ;)

[DiddiLeija]

Ok. I made your suggestions.

[FollowTheProcess]

Cool! Thanks for the contribution @DiddiLeija 🎉

[DiddiLeija]

Thanks to you, @FollowTheProcess, @theacodes, @dhermes and @cjolowicz for all your help!