Skip to content

Conversation

@wyli
Copy link
Contributor

@wyli wyli commented Nov 27, 2020

Signed-off-by: Wenqi Li wenqil@nvidia.com

fixes #852 fixes #927

Description

adds a test util for raising an exception if the execution time exceeds a threshold

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh --codeformat --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@wyli wyli force-pushed the 852-expected-execution-time branch 8 times, most recently from b789665 to 6bdf96f Compare November 29, 2020 18:39
@wyli
Copy link
Contributor Author

wyli commented Nov 29, 2020

/integration-test
test updated integration tests

@wyli wyli force-pushed the 852-expected-execution-time branch 2 times, most recently from 670ba89 to 3405911 Compare November 29, 2020 22:29
@wyli wyli marked this pull request as ready for review November 30, 2020 11:18
@wyli wyli requested review from Nic-Ma and ericspod and removed request for Nic-Ma November 30, 2020 11:18
@wyli
Copy link
Contributor Author

wyli commented Nov 30, 2020

/black

@Nic-Ma Nic-Ma requested a review from IsaacYangSLA December 1, 2020 06:13
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Dec 1, 2020

Hi @IsaacYangSLA ,

Could you please help review this PR to enhance our CICD?

Thanks.

@wyli
Copy link
Contributor Author

wyli commented Dec 1, 2020

Hi @Nic-Ma could you review it as well, this PR also added some testing logic for the integration tests you created earlier

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Dec 1, 2020

OK, sure

Copy link
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

This overall feature looks good to me except some minor comments.
@IsaacYangSLA Is it expected in our CI / CD development plan for the test of execution time?
If you don't have any more comments, we will merge it later.

Thanks.

@wyli wyli force-pushed the 852-expected-execution-time branch from dff4423 to 25623e7 Compare December 2, 2020 11:03
wyli added 7 commits December 2, 2020 08:15
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli force-pushed the 852-expected-execution-time branch from 25623e7 to ba88abb Compare December 2, 2020 13:21
@wyli wyli merged commit 8d61ae2 into Project-MONAI:master Dec 2, 2020
@wyli wyli deleted the 852-expected-execution-time branch April 12, 2021 14:21
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.

Add execute-time test for integration test cases unit tests with expected execution time

2 participants