search

cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
333 stars 94 forks source link

tests: fix flaky test - tests/unit/test_workflow_files.py #5172

Open oliver-sanders opened 2 years ago

oliver-sanders commented 2 years ago

Two of the unit tests are flaky:

FAILED tests/unit/test_workflow_files.py::test_install_workflow__max_depth[foo/foo/-False]
FAILED tests/unit/test_workflow_files.py::test_install_workflow__next_to_flow_file[None-None]

This is because of interaction with symlink dirs:

cylc.flow.exceptions.WorkflowFilesError: Symlink dir target already exists: (/var/tmp/pytest-of-osanders/pytest-3152/popen-gw0/test_install_workflow__next_to2/cylc-run/faden/run1/log ->) /data/users/osanders/cylc-run/faden/run1/log
E           Tip: in future, use 'cylc clean' instead of manually deleting workflow run dirs.

The unit tests should not be creating workflows in the cylc-run directory directly. If needed they should be installed into a test hierarchy as the integration and functional tests are.

Perhaps these tests might be more appropriate as integration tests?

Pull requests welcome!

MetRonnie commented 2 years ago

This is due to the unit tests picking up your global config. IMO they ought not to?

oliver-sanders commented 1 year ago

Yes, the tests should not be picking up the global config (fix export CYLC_CONF_PATH=).

No, tests should not be installing workflows at arbitrary locations like this. They should use the pattern of the functional and integration tests to store these resources in a test hierarchy.

MetRonnie commented 1 year ago

No, tests should not be installing workflows at arbitrary locations like this. They should use the pattern of the functional and integration tests to store these resources in a test hierarchy.

Not sure I understand? The tests are installing workflows in TMPDIR to avoid polluting ~/cylc-run, what's wrong with that?

oliver-sanders commented 1 year ago

The functional and integration tests are careful to install workflows into a hierarchy which is easily visible for debugging, properly managed (e.g. in the event of test failure) and doesn't require any form of mocking to make work.

MetRonnie commented 1 year ago

I'm not sure I see any gain from keeping the run dirs behind after the tests even if they failed. As these are unit tests the only useful information is from the assertion failed message or traceback if the tests errored. By using TMPDIR we don't have to do any cleanup ourselves.