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

WX-1595 Port more tests from papi-common to the GCP Batch backend #7413

Merged
merged 7 commits into from
May 15, 2024

Conversation

AlexITC
Copy link
Collaborator

@AlexITC AlexITC commented Apr 26, 2024

Follows up from #7410

@AlexITC AlexITC requested a review from a team as a code owner April 26, 2024 09:04
@@ -46,6 +46,7 @@ case class GcpBatchWorkflowPaths(workflowDescriptor: BackendWorkflowDescriptor,

private val workflowOptions: WorkflowOptions = workflowDescriptor.workflowOptions

// TODO: Do we actually need this? it seems to be used only by tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if anyone can comment on this, this method seems unnecessary because its only used by the tests, the same occurs in PAPI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like completely dead code in Batch, fine to remove IMO. IntelliJ doesn't even find any references in tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I skipped porting these tests but I wanted to double check before removing the code.

Thanks for looking into it.

@@ -46,6 +46,7 @@ case class GcpBatchWorkflowPaths(workflowDescriptor: BackendWorkflowDescriptor,

private val workflowOptions: WorkflowOptions = workflowDescriptor.workflowOptions

// TODO: Do we actually need this? it seems to be used only by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like completely dead code in Batch, fine to remove IMO. IntelliJ doesn't even find any references in tests.

@aednichols aednichols changed the title Port more tests from papi-common to the GCP Batch backend WX-1595 Port more tests from papi-common to the GCP Batch backend May 14, 2024
@aednichols aednichols enabled auto-merge (squash) May 15, 2024 14:00
@aednichols aednichols merged commit 5d14beb into develop May 15, 2024
34 checks passed
@aednichols aednichols deleted the gcp-batch-add-tests branch May 15, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants