Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove runN if symlink target is gone #4362

Merged
merged 7 commits into from
Aug 19, 2021
Merged

remove runN if symlink target is gone #4362

merged 7 commits into from
Aug 19, 2021

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Aug 17, 2021

These changes close #4323

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.

@wxtim wxtim requested review from MetRonnie, datamel, oliver-sanders and kinow and removed request for MetRonnie and oliver-sanders August 17, 2021 14:03
@wxtim wxtim self-assigned this Aug 17, 2021
@wxtim wxtim added the could be better Not exactly a bug, but not ideal. label Aug 17, 2021
@wxtim wxtim added this to the cylc-8.0b3 milestone Aug 17, 2021
* 'main' of github.com:wxtim/cylc:
  Update CHANGES.md
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One test still failing @wxtim

=========================== short test summary info ============================
FAILED tests/unit/test_workflow_files.py::test_delete_runN[expect1-dirs1] - A...
====== 1 failed, 963 passed, 1 skipped, 1 xfailed, 12 warnings in 36.87s =======

cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
tests/unit/test_workflow_files.py Show resolved Hide resolved
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
@wxtim wxtim requested a review from MetRonnie August 18, 2021 10:51
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fast tests failing GH Actions. Looks to me like the same ones failing on #4361 which was merged into master.

These all pass locally for me. The plus is that bash / bash-docker (5.0) now passing.

Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there should also be a test that runN is not removed if cleaning a non-latest run dir

@wxtim wxtim requested a review from MetRonnie August 18, 2021 15:11
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested manually, all good 👍

@MetRonnie MetRonnie merged commit 26cc709 into cylc:master Aug 19, 2021
@wxtim wxtim deleted the main branch March 22, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
could be better Not exactly a bug, but not ideal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cylc clean leaves behind broken runN symlink
4 participants