# Fieldview Test Utils Second Iteration ###### tags: `cycle 15` <!-- Change to the current cycle number --> - Shaped by: @ricoh - Appetite (FTEs, weeks): 1-2 FTE 1 week - Developers: <!-- Filled in at the betting table unless someone is specifically required here --> - Available for discussion/review: ## Problem The last cycle introduced a new set of test utilities, which reduce the amount of boilerplate required to write many test methods that involve running a FieldView program. Only an initial set of test methods in `test_execution.py` has been converted to using the new utilities: Enough to guide the development and gain confidence in their usability. - old utils: `next_tests.integration_tests.feature_tests.ffront_tests.test_execution` - new utils: `next_tests.integration_tests.feature_tests.cases` - main target for conversion: `next_tests.integration_tests.feature_tests.ffront_tests.test_execution` Test method comparison ```python def test_old_style(fieldview_backend): i_size = ... j_size = ... k_size = ... @field_operator(backend=fieldview_backend) # easy to forget to set the backend def testee(a: Field[[I, J, K], float64], b: Field[[I, J, K], float64]): ... a = np_as_located_field(I, J, K)(numpy.full(..., (i_size, j_size, k_size))) b = np_as_located_field(I, J, K)(numpy.full(..., (i_size, j_size, k_size))) out = np_as_located_field(I, J, K)(numpy.zeros, (i_size, j_size, k_size)) ref = <some numpy expression, possibly involving a.array() and b.array()> testee(a, b, out=out) assert numpy.allclose(out, ref) def test_new_style(cartesian_case): # or unstructured_case @field_operator # backend set automatically, default backend switched off def testee(a: IJKField, b: IJKField) -> IJKField: ... cases.verify_with_default_data( cartesian_case, testee, ref=lambda a, b: <numpy array math expression>, # receives underlying arrats of auto generated input data fields ) def test_new_style_long_form(cartesian_case): # or unstructured_case @field_operator # backend set automatically, default backend switched off def testee(a: IJKField, b: IJKField) -> IJKField: ... tmp = a(IOff[1]) ... a = cases.allocate(cartesian_case, testee, "a").extend({IDim: (0, 1)})() # extend due to shift b = cases.allocate(cartesian_case, testee, "b")() out = cases.allocate(cartesian_case, testee, cases.RETURN)() ref = <some numpy_expression ...> cases.verify(cartesian_case, testee, a, b, out=out, ref=ref) ``` This leaves the test base in a transitional state: some test methods use the old style, some the new. This impacts readability, teachability and maintainability and technical debt negatively. One has to know how to read either. For a newcomer it would not be obvious which to use. Most likely the old and new sets of utilities could be merged together. In addition, not many devs have used the new utils yet, which means their documentation has not been verified to be good enough. Lastly, there is no mention of testing utilities (which is meant here to include test fixtures) in the coding guidelines (`CODING_GUIDELINES.md`). ## Appetite This is intended as a small batch project for [GT4Py Cleanup](https://hackmd.io/-OGkeMJeQT-CqXPQTzlUCw) in cycle 15. It says 1-2 FTEs because discussing the coding guidelines with at least one other person would be beneficial. ## Solution Each of the following priorities is intended to be addressed in a separate PR and should only be considered solved when the PR is merged. If working on this alone, @ricoh will be available for reviews. - [x] Highest priority will be to convert all test methods inside `...feature_tests.ffront_tests.test_execution` to use the new utils. This should not take long, if some difficulty arises in a test method, it might be a rabbit hole. In that case it's ok to skip it with a todo comment summarizing the reason it isn't converted yet. This is definitely worth merging on its own. - [x] The second highest priority is to remove any accidental uses of the default backend in the tests that use the `fieldview_backend` fixture from the old utils module. This can be done by moving the code in `no_default_backend` (new utils) directly into `fieldview_backend` and dealing with any breakages. - [x] Third priority is to scour the tests for (direct) usages of `fieldview_backend` or `reduction_setup` and deciding whether they should be converted too. **Do** split this into many small PRs. **Do not** rely on IDEs to find pytest fixture usages, double check with a text search over all test files. If one of the following criteria is met they should be converted: - the test method directly OR indirectly causes fields to be allocated (perhaps inside a fixture or helper function?) - look for `np_as_located_field` usages. - the test method directly OR indirectly causes a field operator or scan operator or program to be run (could also be happening in a helper somewhere) As many ad-hoc fixtures and helper functions as possible should be removed and replaced by `allocate()` and `verify()` or versions thereof. - [x] Fourth priority will be to merge the old and new utils. While picking a name and location for the resulting test utils module, the `CODING_GUIDELINES.md` should be extended, so that future test utils modules can follow the same thought process. Since the question might come up: the name `cases` for the new test module was based on the idea that details like backend, grid size etc should be summarized in a parametrizable "test case" (there being two types, cartesian and unstructured ones). ## Rabbit holes There may be rare test methods that do something which does not fit into the `allocate-verify` pattern from the new utils. That is fine. The new test utils do not have to cover every case, only the vast majority. No time should be spent on extending them in this project. ## No-gos Do not create one big PR with everything in it.