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

Changed the rose stem functional tests to a more pytest-integration s… #188

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Oct 3, 2022

…tyle.

By separating the command line script into a parser and a rose_stem function I am able to import the rose_stem function and run it without subprocesses. This is good because it is both faster and recognized by codecov.

These changes partially address #157

Requirements check-list

  • 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).
  • Applied any dependency changes to setup.cfg.
  • This PR is itself a change to testing.
  • No change log entry required (invisible to users).
  • No documentation update required.

@wxtim wxtim added this to the 1.1.2 milestone Oct 3, 2022
@wxtim wxtim self-assigned this Oct 3, 2022
@wxtim wxtim force-pushed the convert_subprocess_functional_tests_to_pure_python branch from a7ca960 to 6431826 Compare October 3, 2022 12:06
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.

Coverage up 20%: https://app.codecov.io/gh/cylc/cylc-rose/pull/188

But we're not getting the status here on GitHub. I think @hjoliver might be the only one who can enable the CodeCov app for the repo?

Edit: also on cylc-uiserver

tests/functional/test_rose_stem.py Outdated Show resolved Hide resolved
tests/functional/test_rose_stem.py Outdated Show resolved Hide resolved
@wxtim wxtim force-pushed the convert_subprocess_functional_tests_to_pure_python branch from d908f7c to 3378b0f Compare October 4, 2022 07:35
@wxtim wxtim requested a review from MetRonnie October 4, 2022 07:36
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.

Checked out and tested locally. Quick query about the base branch, should this be going into cylc:master or cylc:1.1.x?

@MetRonnie
Copy link
Member

IMO both, as it's just tests that have been affected

@oliver-sanders oliver-sanders modified the milestones: 1.1.2, 1.2.0 Oct 11, 2022
@oliver-sanders oliver-sanders merged commit 7464430 into cylc:master Oct 11, 2022
@oliver-sanders
Copy link
Member

Let's get it into 1.2.0 as it is raised on master. No real need to backport.

@wxtim wxtim deleted the convert_subprocess_functional_tests_to_pure_python branch October 11, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants