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

More specific testing for parallel ops formatting #71

Merged
merged 3 commits into from
Dec 15, 2023
Merged

More specific testing for parallel ops formatting #71

merged 3 commits into from
Dec 15, 2023

Conversation

Asa-Kosto-QTM
Copy link
Collaborator

No description provided.

qartik
qartik requested changes Dec 15, 2023
View reviewed changes
Copy link
Member

@qartik qartik left a comment

Choose a reason for hiding this comment

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

Looking good but can be better. Left a suggestion that you can apply to other tests as well.

Also, consider creating a utility file, where you can keep common functions such as get_phir_json and generalize them to take parameters instead of duplicating them.

tests/test_parallelization.py Outdated Show resolved Hide resolved
tests/test_parallelization.py Outdated Show resolved Hide resolved
qartik
qartik reviewed Dec 15, 2023
View reviewed changes
tests/test_parallel_tk2.py Outdated Show resolved Hide resolved
@qartik qartik mentioned this pull request Dec 15, 2023
Closed
qartik
qartik reviewed Dec 15, 2023
View reviewed changes
tests/test_utils.py Outdated Show resolved Hide resolved
qartik
qartik reviewed Dec 15, 2023
View reviewed changes
tests/test_parallel_tk2.py Outdated Show resolved Hide resolved
qartik
qartik reviewed Dec 15, 2023
View reviewed changes
tests/test_utils.py Outdated Show resolved Hide resolved
qartik
qartik requested changes Dec 15, 2023
View reviewed changes
Copy link
Member

@qartik qartik left a comment

Choose a reason for hiding this comment

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

Looking good, just some minor changes needed.

qartik
qartik reviewed Dec 15, 2023
View reviewed changes
tests/test_parallel_tk2.py Outdated Show resolved Hide resolved
@Asa-Kosto-QTM Asa-Kosto-QTM merged commit 63830bc into main Dec 15, 2023
2 checks passed
@qartik qartik deleted the parallel-ops-testing branch December 15, 2023 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qartik qartik qartik 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.
2 participants
@Asa-Kosto-QTM @qartik