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

test_Data workflow with skips to monitor Dask migration #244

Merged

Conversation

sadielbartholomew
Copy link
Member

@sadielbartholomew sadielbartholomew commented Aug 13, 2021

I am about to put up some more interesting PRs to make headway with 'Daskifying' the data-related methods in turn for #182, but before I start doing that in earnest I want to ensure we can track the migration progress in terms of the tests that pass, notably for (self-)review to:

  • indicate progress for a given PR by highlighting extra tests that pass due to a given change; and
  • ensure tests that do pass due to progress previously made aren't broken by any proposed changes in a new PR.

So in this PR I have:

  • added skips across test_Data to mark tests not yet made to pass, that can all be switched on or off by a new flag TEST_DASKIFIED_ONLY;
  • added a new workflow specifically specifically for the LAMA-to-dask migration work to trigger Github Actions to run jobs that run the test_Data module and not the whole test suite, because the first goal is to get the data modules working, where metadata tests will fail on data issues until that is complete.

@sadielbartholomew
Copy link
Member Author

The new workflow clearly didn't trigger, I try to get it to start on Monday and go from there.

@sadielbartholomew sadielbartholomew marked this pull request as draft August 16, 2021 03:20
@sadielbartholomew
Copy link
Member Author

Using an open-close to see if that will result in a trigger of the new workflow...

@sadielbartholomew
Copy link
Member Author

Triggering worked 🎉 With the new test skips, I want to get the jobs to pass before I can consider the new workflow to be ready, so let's see how they fare...

@sadielbartholomew
Copy link
Member Author

sadielbartholomew commented Aug 16, 2021

Most of the Ubuntu jobs have passed, largely due to the intentional skips on the test_Data they ultimately run:

...

----------------------------------------------------------------------
Ran 73 tests in 0.221s

OK (skipped=66)

...

with the Mac OS jobs generally taking longer and being cancelled by the one failure on the Ubuntu with Python 3.6 job. I have just checked and Dask no longer supports Python 3.6 (https://github.com/dask/dask/blob/main/setup.py#L71), hence the lone test failure on test_Data___setitem__ for that version only due to a NotImplementedError: Item assignment with <class 'tuple'> not supported. I will remove Python 3.6 from the job matrix in the next commit, and hopefully both the Ubuntu and Mac OS jobs will then all pass.

@sadielbartholomew
Copy link
Member Author

Re-triggering via open-close...

@sadielbartholomew sadielbartholomew removed the request for review from davidhassell August 16, 2021 15:26
@sadielbartholomew sadielbartholomew changed the title Add test_Data skips to help monitor Dask migration test_Data workflow with skips to monitor Dask migration Aug 16, 2021
@sadielbartholomew sadielbartholomew marked this pull request as ready for review August 16, 2021 22:59
@sadielbartholomew sadielbartholomew merged commit 9fea9ad into NCAS-CMS:lama-to-dask Aug 16, 2021
@sadielbartholomew sadielbartholomew deleted the lama-to-dask-1 branch August 16, 2021 23:03
@sadielbartholomew
Copy link
Member Author

sadielbartholomew commented Aug 16, 2021

Sorry @davidhassell I initially added you to review and then realised there is nothing much to review here so removed you as a reviewer, but please do read the opening comment so you know about the skipping of some test_Data test methods which will be incrementally un-skipped as more progress is made, and the new workflow to run test_Data only for PRs to the lama-to-dask development branch for #182.

@davidhassell davidhassell added the dask Relating to the use of Dask label Oct 5, 2021
@davidhassell davidhassell added this to the 3.14.0 milestone Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dask Relating to the use of Dask
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants