search

CBICA / CaPTk

Cancer Imaging Phenomics Toolkit (CaPTk) is a software platform to perform image analysis and predictive modeling tasks. Documentation: https://cbica.github.io/CaPTk
https://www.cbica.upenn.edu/captk
Other
179 stars 64 forks source link

Azure not catching failing tests #986

Open ashishsingh18 opened 4 years ago

ashishsingh18 commented 4 years ago

Describe the bug Training module tests are failing on Azure. However Azure checks are being reported as successful.

Expected behavior Azure should report failure.

Screenshots If applicable, add screenshots to help explain your problem. image

image

CaPTk Version: latest master

MarkBergman-cbica commented 4 years ago

Need to distinguish between:

  1. why these tests (TrainingModuleTest10Folds) fail?
  2. is Azure behaving as expected -- did any test failure cause Azure to declare a failure in the past?
AlexanderGetka-cbica commented 4 years ago

Currently we aren't actually running the tests for either self-hosted or end-to-end builds. @PhucNgo1711 what was the reason these were disabled if they weren't failing the build pipeline?

PhucNgo1711 commented 4 years ago

Currently we aren't actually running the tests for either self-hosted or end-to-end builds. @PhucNgo1711 what was the reason these were disabled if they weren't failing the build pipeline?

Hmm I think it was commented out originally for space and time concern when we were using the cloud VMs. I guess we can put the test tasks back. But if they fail Azure won't be able to catch them still because they are just logs.

AlexanderGetka-cbica commented 4 years ago

Yeah, I saw that even if the tests fail ctest returns a normal exit code, so azure doesn't pick it up as failing. I was thinking I could parse over the output and check for failures, feed that into an Azure variable "AnyTestsFailed" and then have a Fail task that runs conditionally with respect to that variable. Speaking with @MarkBergman-cbica it became clear that it'd probably be best to have some way to have some tests required, others optional for Azure to succeed. But since this might be difficult to change now, we should probably at least print test output and have every test be optional, especially since we already aren't really factoring in the unit tests.

sarthakpati commented 4 years ago

@AlexanderGetka-cbica: could you try incorporating this solution during the unit testing phase?

Also, I saw that there are no unit tests getting built on the self-hosted machines, which is probably something we want (they can be disabled on the Azure-hosted machines, however).

MarkBergman-cbica commented 4 years ago

In the message dated: Mon, 21 Sep 2020 12:42:21 -0700, The pithy ruminations from Sarthak Pati on [[External] Re: [CBICA/CaPTk] Azure not catching failing tests (#986)] were: => @AlexanderGetka-cbica: could you try incorporating this solution during the unit testing phase? => => Also, I saw that there are no unit tests getting built on the self-hosted machines, which is probably something we want (they can be disabled on the Azure-hosted machines, however).

More than just something we want, I believe that was one of the primary reasons for doing self-hosting.

Mark

=> => -- => You are receiving this because you were mentioned. => Reply to this email directly or view it on GitHub: => https://github.com/CBICA/CaPTk/issues/986#issuecomment-696330723

-- Mark Bergman voice: 215-746-4061
mark.bergman@pennmedicine.upenn.edu fax: 215-614-0266 http://www.med.upenn.edu/cbica/ IT Technical Director, Center for Biomedical Image Computing and Analytics Department of Radiology University of Pennsylvania

AlexanderGetka-cbica commented 4 years ago

I am concerned about the amount of time the full test suite would take running on the self-hosted agents. The TrainingModule tests may run quickly on the cluster, but on the machines we're running agents on it could take many hours.

Azure does allow us to set up a "release" pipeline separately from the build pipeline. We could use that as a way to run tests separately from every build, perhaps at set time intervals (test from master biweekly, for example). Unfortunately this would not prevent a build that builds successfully but breaks functionality. As an alternative we could limit what tests are being run on every build -- we can filter tests out by name from ctest, after all. In fact, we can even do rudimentary branching on Azure, so for example manual runs could trigger the full test suite while PR runs and midnight builds only run a subsection of them. But I'm not sure which works best.

We should discuss tomorrow about what tests are appropriate to run regularly. In the meantime I'm going to try to look into how other projects handle things like this.

MarkBergman-cbica commented 4 years ago

In the message dated: Mon, 21 Sep 2020 14:11:12 -0700, The pithy ruminations from Alexander Getka on [[External] Re: [CBICA/CaPTk] Azure not catching failing tests (#986)] were: => I am concerned about the amount of time the full test suite would take running on the self-hosted agents. The TrainingModule tests may run quickly on the cluster, but on the machines we're running agents on it could take many hours. =>

What's the problem with that?

I was under the impression that we could designate agents as "optional", so those long-running builds wouldn't be required for a PR to pass.

=> Azure does allow us to set up a "release" pipeline separately from the build pipeline. We could use that as a way to run tests separately from every build, perhaps at set time intervals (test from master biweekly, for example). Unfortunately this would not prevent a build that builds successfully but breaks functionality. As an alternative we could limit what tests are being run on every build -- we can filter tests out by name from ctest, after all.

Maybe nightly, to catch functional failures more quickly.

=> In fact, we can even do rudimentary branching on Azure, so for example manual runs could trigger the full test suite while PR runs and midnight builds only run a subsection of them. But I'm not sure which works best. => => We should discuss tomorrow about what tests are appropriate to run regularly. In the meantime I'm going to try to look into how other projects handle things like this. => => -- => You are receiving this because you were mentioned. => Reply to this email directly or view it on GitHub: => https://github.com/CBICA/CaPTk/issues/986#issuecomment-696379147

-- Mark Bergman voice: 215-746-4061
mark.bergman@pennmedicine.upenn.edu fax: 215-614-0266 http://www.med.upenn.edu/cbica/ IT Technical Director, Center for Biomedical Image Computing and Analytics Department of Radiology University of Pennsylvania