tiatoolbox icon indicating copy to clipboard operation
tiatoolbox copied to clipboard

Published 20 hours ago • verified publisher icon TissueImageAnalytics

Missing tests

Open DavidBAEpstein opened this issue 2 years ago • 5 comments

Description

Both Issues #351 and #352 indicate that our testing regime is inadequate. For now, I will discuss only a test associated with #352. I would do this in bash as follows.

  1. Produce a random name for an environment, for example ran357
  2. Make a copy of requirements.dev.conda.yml with name ran357.yml
  3. Use sed to replace tiatoolbox.dev with ran357
  4. Save error message from 'conda env create --file ran357.yml'
  5. Assert error message is empty Please let me know how to proceed, as I have never before written any test. Our onenote notebook assumes that only python files are being tested.

Possibly the other requirements files could be similarly tested.

The other failure of requirements.dev.conda.yml that I experienced was the failure of 'make html'. Not much point in this case of checking the error output. One could check that docs/_build/html exists and is not empty.

DavidBAEpstein avatar May 30 '22 10:05 DavidBAEpstein

Hi David, Re #352, our tests run using the pip installation file. The conda installation requires a lot of time on readthedocs and travis and therefore is not automatically tested. We should have however tested the conda requirements file before making those changes in the conda file manually. I have made a note of this and in future for any conda requirements update we will need to manually check the requirements file on Windows/Mac/Linux.

To resolve #352, I have created #353. Please can you check this file as this should fix the problem.

shaneahmed avatar May 30 '22 11:05 shaneahmed

Re #351, we wanted to setup automated testing for jupyter notebooks but we haven't added those yet. We can discuss this in the meeting.

shaneahmed avatar May 30 '22 11:05 shaneahmed

Good. If we test jnbs, we need to take into account that several of them invite the user to CHANGE the source code (Changing ON_GPU from True to False and vice versa). Both versions need to be tested. The default should be ON_GPU = True, and this is what should be held by the master branch.

The ON_GPU = False version should be automatically generated and then checked separately. However, the False version could take a long time to run. Another problem, this time with ON_GPU = True, is that @vqdang tells me that, in certain circumstances, the mechanism used by Python when the number of workers is changed, can lead to overflow in some systems but not in others. I believe that several members of the TIAToolbox team are aware of this.

DavidBAEpstein avatar May 30 '22 13:05 DavidBAEpstein

The PR #392 may help address some of these issues.

John-P avatar Jul 04 '22 17:07 John-P

It could have saved me a lot of time and puzzlement, though the second time nbsphinx was omitted I realized immediately.

On 4 Jul 2022, at 18:01, John Pocock @.@.>> wrote:

The PR #392https://github.com/TissueImageAnalytics/tiatoolbox/pull/392 may help address some of these issues.

— Reply to this email directly, view it on GitHubhttps://github.com/TissueImageAnalytics/tiatoolbox/issues/354#issuecomment-1174007837, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFIQKBH432PVHWMOZR4UPUDVSMKGBANCNFSM5XKFEYKA. You are receiving this because you are subscribed to this thread.Message ID: @.***>

DavidBAEpstein avatar Jul 04 '22 18:07 DavidBAEpstein

We have added tests for Jupyter notebooks where it was possible.

shaneahmed avatar Aug 26 '22 09:08 shaneahmed