Skip to content

Conversation

@greenc-FNAL
Copy link
Contributor

@greenc-FNAL greenc-FNAL commented Dec 19, 2025

The failure to catch exceptions (leading to a call to terminate()) was preventing the accumulation of test coverage statistics for failure cases.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds exception handling around the phlex::experimental::run() call in the main function to ensure graceful program termination and proper coverage data generation during testing. The changes wrap the run call in a try-catch block that handles both standard exceptions and unknown exceptions.

  • Wraps phlex::experimental::run() in try-catch blocks to catch exceptions before program exit
  • Adds separate handlers for std::exception and catch-all for unknown exceptions
  • Returns exit code 1 for exception cases to indicate failure

@knoepfel
Copy link
Member

@phlexbot format

@github-actions
Copy link
Contributor

No automatic cmake-format fixes were necessary.

@github-actions
Copy link
Contributor

Automatic clang-format fixes pushed (commit 94b8230).
⚠️ Note: Some issues may require manual review and fixing.

@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
phlex/app/phlex.cpp 37.50% 5 Missing ⚠️

❌ Your patch status has failed because the patch coverage (37.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

@@            Coverage Diff             @@
##             main     #211      +/-   ##
==========================================
- Coverage   82.13%   81.92%   -0.22%     
==========================================
  Files         118      118              
  Lines        2211     2218       +7     
  Branches      353      353              
==========================================
+ Hits         1816     1817       +1     
- Misses        262      267       +5     
- Partials      133      134       +1     
Flag Coverage Δ
unittests 81.92% <37.50%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
phlex/app/phlex.cpp 61.66% <37.50%> (-4.38%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec29fea...94b8230. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@knoepfel knoepfel merged commit 4552cd0 into main Dec 19, 2025
35 of 36 checks passed
@knoepfel knoepfel deleted the maintenance/ensure-graceful-phlex-app-exit branch December 19, 2025 01:03
knoepfel pushed a commit to wlav/phlex that referenced this pull request Dec 19, 2025
…neration (Framework-R-D#211)

* Catch exceptions in main to ensure graceful exit and coverage data generation

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
greenc-FNAL added a commit that referenced this pull request Dec 19, 2025
* proof-of-concept Python support using converter nodes

* add wrapper codes to the library

* code cleanup (clang-format) and simplifications

* clang-format fixes and disable it for the Python Type definitions

* clang-format fixes of the header files

* extend supported types to a couple more builins and retrieve information from annotations

* fix cmake formatting

* move py:phlex property underneath the HAS_CPPYY block

* add missing registration helper module

* Python exception -> std::runtime_error

* observer to check adder algorithm output

* move initial GIL release later and make sure it only happens once

* a function with no configured output becomes an observer

* remove spurious printout

* change configuration lookup failures into Python exceptions

* make sure that the adder sum result is non-zero

* improve testing by adding an observer that asserts the algorithm output

* move the GIL RAII to the common wrapper header file for reuse

* update to new registration API

* fix vector indexing error if no outputs provided

* add error helper to pymodule.so

* fix cmake formatting to conform to the rules

* add a way to pass configuration to python modules

* support callable instances

* trivial demonstrator of std::vector<int> input to a Python algorithm

* add vector test files

* make python module registration resemble the C++ one more closely

* simplify life-times by letting the node take a reference to the registered algorithm

* rename "pymodule" property to "pyplugin"

* formatting fixes (clang-format getting confused by PyObject_HEAD)

* vector support goes through Numpy views for now, so add that depedency

* add a lifeline object to tie life times of handles and views onto them

* use numpy views instead of array copies to handle std::vector

* explicitly collect tests based on activation before setting properties

* protect all of import_numpy to prevent an "unused variable" warning

* clang format remove empty line

* another hiding of unused variables attempt to make coverage happy

* one more attempt to compile w/o errors if numpy isn't installed

* add additional vector types and use numpy.typing in the annotations

* move python support module from test to plugins directory

* split up the generic register into transform and observe registration

* fix typos and errors/missing comments

* clang-format fixes

* enable the "python" subdirectory in "plugins"

* simplify retrievel of configuration values in python by using the declared kind from json

* expose kind() for the held json values to simplify conversion to Python

* Range of cosmetic fixes.

* clang-format fixes

* return false in initalize on failure to initialize custom types

* add an internal cache for python-side configuration

* rename kind -> prototype_internal_kind

* clang format fixes

* allow an annotation of `None` as "no output"

* add a test to check mapping of configuration types

* uniquely tie converter nodes to algorithms for lifetime management

* cleanup

* upgrade to new registration interface (note: DOES NOT WORK)

* upgrade to new interface

* clang-format fix

* cmake formatting fixes

* add module-level docstrings to all python modules

* fix Phlex plugin search path

* add support for numpy -> std::vector returns (through copying)

* use the "job" layer as a default for now until a python representation of queries exists

* ruff fixes

* fix formatting for the case where numpy isn't installed

* add a way for testing failures/error paths for code coverage

* Catch exceptions in main to ensure graceful exit and coverage data generation (#211)

* Catch exceptions in main to ensure graceful exit and coverage data generation

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: Chris Green <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants