FIX: Only remove temporary folder if it is present

[chrisjsewell]

Surprised no one has had this issue before, but on OSX at least this shutil.rmtree fails if retrieved_temporary_folder is None:

TypeError: lstat: path should be string, bytes or os.PathLike, not NoneType

[codecov]

Codecov Report

Merging #4379 into develop will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #4379      +/-   ##
===========================================
+ Coverage    79.29%   79.30%   +0.01%     
===========================================
  Files          476      476              
  Lines        34958    34959       +1     
===========================================
+ Hits         27718    27720       +2     
+ Misses        7240     7239       -1     

Flag	Coverage Δ
#django	73.17% <100.00%> (+0.01%)	⬆️
#sqlalchemy	72.38% <100.00%> (-<0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/engine/processes/calcjobs/calcjob.py	82.76% <100.00%> (+0.05%)	⬆️
aiida/transports/plugins/local.py	81.29% <0.00%> (-0.25%)	⬇️
aiida/engine/daemon/client.py	73.57% <0.00%> (+1.15%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9460e4e...bdca007. Read the comment docs.

[sphuber]

This line

aiida-core/aiida/engine/processes/calcjobs/calcjob.py

Line 315 in c6bca06

shutil.rmtree(retrieved_temporary_folder, ignore_errors=True)

might then also bite us in the ass on OS at some time. For now it is never None because the temporary directory is always created and passed. Why is Mac OS so shit? ;)

[chrisjsewell]

Why is Mac OS so shit? ;)

You mean because it does the right thing 😉 bet this would fail a type check

[chrisjsewell]

BTW, as mentioned in #4375 (comment), I was just trying to get all the tests to pass locally;

The tests/cmdline/commands/test_daemon.py::TestVerdiDaemon tests fail, is that expected, if you are just using the temporary profile creation?

[chrisjsewell]

Also, after the passing tests finish, I get 100s of the same message:

Exception ignored in: <generator object create_task.<locals>.run_task at 0x7f84deb94e50>
Traceback (most recent call last):
  File "/Users/chrisjsewell/Documents/GitHub/aiida_core_develop/.tox/py37-django/lib/python3.7/site-packages/plumpy/futures.py", line 102, in run_task
  File "/Users/chrisjsewell/Documents/GitHub/aiida_core_develop/.tox/py37-django/lib/python3.7/contextlib.py", line 158, in __exit__
AttributeError: 'NoneType' object has no attribute 'exc_info'

which doesn't seem healthy 😬

[sphuber]

Also, after the passing tests finish, I get 100s of the same message:

This has been fixed in the latest plumpy. Just install the latest patch version and those will go away.

[sphuber]

BTW, as mentioned in #4375 (comment), I was just trying to get all the tests to pass locally;

The tests/cmdline/commands/test_daemon.py::TestVerdiDaemon tests fail, is that expected, if you are just using the temporary profile creation?

Cannot say without having a closer look. Would it make sense to hold of with merging this and trying to tackle the rest as well and go in one go?

[chrisjsewell]

Would it make sense to hold of with merging this and trying to tackle the rest as well and go in one go?

Up to you

For the sphinx ones, not sure if this will solve it, but it would probably be better to use the "proper" sphinx test fixtures: https://github.com/sphinx-doc/sphinx/blob/3.x/sphinx/testing/fixtures.py

For the TestVerdiDaemon ones, it would be helpful to know if pytest tests/cmdline/commands/test_daemon.py and/or tox -- tests/cmdline/commands/test_daemon.py simply passes on Linux, or if you need to use a "pre-created" profile like in .github/workflows/ci-code.yml:jobs:tests

[sphuber]

For the TestVerdiDaemon ones, it would be helpful to know if pytest tests/cmdline/commands/test_daemon.py and/or tox -- tests/cmdline/commands/test_daemon.py simply passes on Linux, or if you need to use a "pre-created" profile like in .github/workflows/ci-code.yml:jobs:tests

Nope, with on the fly profile this also fails on Ubuntu. However with existing profile this works just fine.

[chrisjsewell]

I'm merging this, since its a trivial thing