Tox: [BUG] Backwards incompatible API change in 3.8.0

[drdavella]

This was noticed in the tox-conda package (see https://github.com/tox-dev/tox-conda/issues/15) although it seems like it might actually be an issue in tox itself with the latest release. Here is the error that occurs:

Traceback (most recent call last):
  File "/Users/ddavella/miniconda3/envs/asdf-tox/bin/tox", line 10, in <module>
    sys.exit(cmdline())
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/session/__init__.py", line 42, in cmdline
    main(args)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/session/__init__.py", line 63, in main
    retcode = session.runcommand()
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/session/__init__.py", line 187, in runcommand
    return self.subcommand_test()
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/session/__init__.py", line 215, in subcommand_test
    run_sequential(self.config, self.venv_dict)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/session/commands/run/sequential.py", line 9, in run_sequential
    if venv.setupenv():
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/venv.py", line 582, in setupenv
    status = self.update(action=action)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox/venv.py", line 240, in update
    self.hook.tox_testenv_create(action=action, venv=self)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/pluggy/hooks.py", line 289, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/pluggy/manager.py", line 68, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/pluggy/manager.py", line 62, in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/pluggy/callers.py", line 208, in _multicall
    return outcome.get_result()
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "/Users/ddavella/miniconda3/envs/asdf-tox/lib/python3.7/site-packages/tox_conda/plugin.py", line 100, in tox_testenv_create
    venv.session.make_emptydir(venv.path)
AttributeError: 'VirtualEnv' object has no attribute 'session'

This seems like a breaking API change? There's nothing in the change log about this.

[gaborbernat]

Hello, make_emptydir was never meant to be a public API or publishing the session that is. We probably should have pushed this as 4.0 to be fair, though as a fix please use `cleanup_venv`` instead https://github.com/tox-dev/tox/blob/master/src/tox/venv.py#L682 (this has the advantage of being parallel compatible)

We never really published an official API for the venv/action passed in into hooks. So sadly at the moment either everything is public (which hinders progress when implementing new features such as the auto-provisioning, refactoring logging); part of the improvements we moved around a few functions, hence the error above.

GitHub

tox-dev/tox

Command line driven CI frontend and development task automation tool - tox-dev/tox

[gaborbernat]

We'll fix this within tox-conda 👍 thanks for the report!

[drdavella]

Thanks for the quick response and the effort on this.

Just a comment, though: "never meant to be" is not the same as "was not". This was a public attribute of a class that was part of a public API. Just because there wasn't a docstring doesn't mean it wasn't public. The burden is on tox developers to maintain backwards compatibility or provide workarounds.

We will potentially have the issue of plugins that will not be backwards compatible with older versions of tox once this fix is implemented.

[gaborbernat]

@ddriddle Yeah I'm aware. As far as all that's not private is public I'm not sure. pip, for example, is not public even if not hidden behind a _pip package... Granted we should have stated this somewhere if we expect similar publicity. We'll tackle downstream issues raised due to this as they come up. Thanks for using the tool and sorry for the trouble it caused you! Hope the benefits introduced via this will be worth for you too down the line.

[cjerdonek]

Granted we should have stated this somewhere if we expect similar publicity.

@gaborbernat After poking around the tox docs, I found that tox did put a disclaimer in its docs about the API. For example, the documentation for tox_testenv_create() says:

This api is experimental due to the unstable api of tox.venv.VirtualEnv.

[drdavella]

@cjerdonek that's fine, but how is a developer supposed to see that unless they just happen to look at the doc for that particular function? That comment clearly belongs in the docstring of the VirtualEnv class itself or else it's basically useless.

I'm not trying to make a bigger deal here than it actually is, but it is completely reasonable for developers to assume (as they did) that a public attribute of a public API would not change without warning between minor versions.

Also, even if the API is unstable, that is no reason to change these things without any discussion of it in the change log. As a result, I woke up to a bunch of failing CI jobs this morning and had to take time out of my day to fix them and resolve the issue.

[gaborbernat]

@drdavella https://github.com/tox-dev/tox-conda/pull/16 I believe this should fix your failing tests, feel free to check it and release it 👍 (as I see you're the maintainer on pypi); and sorry again for the troubles caused (mia culpa) - really should have put a note about it inside the changelog 👍

[cjerdonek]

@drdavella I'm not saying I don't agree with you. I was just providing some new information I hadn't seen anyone else mention, namely that the docs say a number of these API functions are unstable / experimental. (Also, the info in the docs is auto-generated from docstrings so it's also in the code.)

[drdavella]

@gaborbernat, @cjerdonek thanks for the comments, and no need for apologies. Just want to say sorry myself because sometimes tone doesn't come across very well in text. I'm not really that upset 😄.

Again, not to make a mountain out of a molehill, but I just basically wanted to raise awareness going forward. I have been on the other side of this in terms of making changes in packages that I maintain that break other people's code, but it's always good to keep in mind how changes might affect other users, and the best way to communicate changes.

Thanks again for the timely response.