Skip to content

Conversation

@bonachea
Copy link
Member

This PR begins with one commit containing the (rebased & squashed) commits authored by @rouson in rouson#1.

The subsequent commits are my own cleanups and refinements.

Highlights:

  1. Veggies and its dependencies are now entirely replaced with Julienne (currently v3.4.0)
  2. Test fixtures have been reordered somewhat, sorting by increasing complexity and feature coverage
  3. Some string literals in the tests have been adjusted for consistency across tests
  4. Tests using the incremental diagnosis idiom now use a helper macro and strictly follow a specific convention (added to the documented coding conventions)
  5. A new internal FPM_DRIVER variable helps to slightly improve robustness of the exit tests

The intent is there should be NO changes to the PRIF calls being made, and logic of what's actually being tested should be entirely equivalent to main.

Copy link
Collaborator

@ktras ktras left a comment

Choose a reason for hiding this comment

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

Only reviewed the docs changes so far, but submitting the review to ensure I don't lose the review comments. Will continue to review rest of PR.

@ktras ktras added the release-blocker Must be resolved before next release label Dec 10, 2025
@bonachea bonachea added priority=high and removed release-blocker Must be resolved before next release labels Dec 10, 2025
@ktras ktras added the release-blocker Must be resolved before next release label Dec 10, 2025
Copy link
Collaborator

@ktras ktras left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member Author

@bonachea bonachea left a comment

Choose a reason for hiding this comment

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

Some issues discovered through end-to-end read

Copy link
Collaborator

@ktras ktras left a comment

Choose a reason for hiding this comment

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

Added code suggestion to bump Julienne release now that 3.5.0 is available.

rouson and others added 19 commits December 12, 2025 18:12
 This is a combination of 12 commits:

refactor(prif_rma_test): switch to julienne

refactor(prif_strided_test): switch to julienne

refactor(prif_allocate_test): switch to julienne

refactor(prif_event_test): switch to julienne

refactor(prif_teams_test): switch to julienne

refactor(prif_image_index_test):switch to julienne

refactor(prif_atomic_test): switch to julienne

refactor(prif_error_stop_test): switch to julienne

refactor(prif_stop_test): switch to julienne

chore(prif_init ): rm redundant test module

chore: reorder tests

This commit reorders the tests to match the test-execution ordering
of commit 6db489: roughly simplest to most complicated.

test(driver): add all remaining julienne tests
Add some comments with rationale for the ordering
* Upgrade unit_test_parameters_m getenv query to include verbose-mode reporting
* Add an FPM_DRIVER envvar to parameterize the fpm invcations in the stop tests

This improves robustness of the stop tests when sharing a build directory between different configurations.
It turns out we still need this hack for Julienne, to ensure
the `--contains` filter works correctly.

For details see Julienne issue 142.
…om gfortran

Examples seen from gfortran 14.2:
```
test/prif_event_test.F90:61:10:

   61 |         ])
      |          ^
Warning: '_F.DA0.offset' is used uninitialized [-Wuninitialized]
test/prif_event_test.F90:357:28:

  357 | end module prif_event_test_m
      |                            ^
note: '_F.DA0' declared here
test/prif_event_test.F90:61:10:

   61 |         ])
      |          ^
Warning: '_F.DA0.dim[0].lbound' is used uninitialized [-Wuninitialized]
test/prif_event_test.F90:357:28:

  357 | end module prif_event_test_m
      |                            ^
note: '_F.DA0' declared here
test/prif_event_test.F90:61:10:

   61 |         ])
      |          ^
Warning: '_F.DA0.dim[0].ubound' is used uninitialized [-Wuninitialized]
```
New ALSO macros allow for more concise expression of the incremental
diagnosis idiom, increasing readability of the test without degrading
diagnostic output.

BEFORE:
```
diag = diag .also. (.expect. (.not. c_associated(c1))) // "expected .not. c_associated(c1)"
diag = diag .also. (query_size .equalsExpected. data_size) // "invalid prif_size_bytes"
```

AFTER:
```
ALSO(.expect. (.not. c_associated(c1)))
ALSO2(query_size .equalsExpected. data_size, "invalid prif_size_bytes")
```

The use of a macro also makes it simple to automatically include exact line number
information in the diagnostic output of a failure, example output:

```
   FAILS  on allocating, using and deallocating an integer array coarray with a corank of 2.
      diagnostics:
        expected to be true
./test/prif_allocate_test.F90:176: FAILED: expression: (.expect.(.not. c_associated(c1)))
```
* New `test_diagnosis_t` copy constructor allows the `ALSO` macros to accept
  either `test_diagnosis_t` or default logical
* This in turn allows removal of many uses of `.expect.`
Copy link
Collaborator

@ktras ktras left a comment

Choose a reason for hiding this comment

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

Updated changes LGTM.

@bonachea bonachea merged commit 5400345 into BerkeleyLab:main Dec 13, 2025
31 checks passed
@bonachea bonachea deleted the julienne-two branch December 13, 2025 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority=high release-blocker Must be resolved before next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants