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.

Sign up for GitHub

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

Jump to bottom

[test] add cli tests #350

Merged
merged 1 commit into from
Jul 17, 2023
Merged

[test] add cli tests #350

merged 1 commit into from
Jul 17, 2023

Conversation

sergkudinov
Copy link
Contributor

@sergkudinov sergkudinov commented Jul 5, 2023 •
edited

Which issue(s) this PR fixes

Fixes #332

Additional comments

Agreements

@sergkudinov sergkudinov changed the title test: add cli tests [test] add cli tests Jul 5, 2023
@sergkudinov sergkudinov mentioned this pull request Jul 5, 2023
Open
@sergkudinov sergkudinov mentioned this pull request Jul 6, 2023
Closed
rpignolet
rpignolet requested changes Jul 7, 2023
View reviewed changes
tdp/cli/commands/plan/test_reconfigure.py Outdated Show resolved Hide resolved
tdp/cli/commands/plan/test_dag.py Show resolved Hide resolved
tdp/cli/commands/plan/test_dag.py Outdated Show resolved Hide resolved
@sergkudinov sergkudinov force-pushed the 332-test-cli branch 2 times, most recently from 3143e46 to 5c4e020 Compare July 7, 2023 15:40
@sergkudinov
Copy link
Contributor Author

@rpignolet
ready fo re-review

PaulFarault
PaulFarault requested changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@PaulFarault PaulFarault left a comment

Choose a reason for hiding this comment

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

Why aren't the tests involving the plan commands in commands/plan?

tdp/cli/conftest.py Outdated Show resolved Hide resolved
tdp/cli/commands/test_init.py Outdated Show resolved Hide resolved
@PaulFarault
Copy link
Contributor

Can't the conftest be moved to the commands directory?

Is it used elsewhere in the cli? If no we may want to move it too.

@sergkudinov
Copy link
Contributor Author

sergkudinov commented Jul 17, 2023 •
edited

@PaulFarault

So far, it is is used only in commands, moving, later on we can move it back if neccessary.

@sergkudinov sergkudinov merged commit 7025ebc into master Jul 17, 2023
2 checks passed
@sergkudinov sergkudinov deleted the 332-test-cli branch July 17, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rpignolet rpignolet rpignolet approved these changes

@PaulFarault PaulFarault PaulFarault approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[test] Add test for the CLI
3 participants
@sergkudinov @PaulFarault @rpignolet