Grow Ric*Feature command-feature test coverage - #1013
Open
magnesj wants to merge 22 commits into
Open
Conversation
Add a new ResInsight-featuretests GoogleTest executable that boots a headless (offscreen) RiaGuiApplication and exercises the object-selection system through the Ric*Feature command layer: - Enablement sweep: one parameterized test per registered feature that calls isCommandEnabled() with an empty selection, isolating any crash to the offending feature id. - Execution sweep with an in-memory model builder, modal-dialog watchdog, and denylist. Disabled by default because features that assume a valid GUI selection context trip CVF_ASSERT/CAF_ASSERT that abort or hang and cannot be isolated per-feature without subprocess isolation. - Curated deep tests for RicDeleteWellPathFeature and RicNewViewFeature asserting real model changes. Fix a null-pointer dereference in RicPlaceThermalFractureUsingTemplateDataFeature::selectedThermalFracture() surfaced by the enablement sweep: it dereferenced the result of selectedItemOfType<RimWellPathFracture>() without a null check. Add RiaRegressionTestRunner::setRunningRegressionTests() so the test main can enable the dialog-suppression code paths.
… well path Add RicCloseCaseFeature-Test asserting a selected Eclipse case is removed from the project. Extend RicNewViewFeature-Test to cover creating a view from a selected view, and RicDeleteWellPathFeature-Test to cover deleting one of several well paths and the empty-selection disabled state.
Add RiaFeatureTestTreeView, an RAII helper that builds a headless project tree view and registers it as the active tree view via RiaFeatureCommandContext. This lets tree-driven features resolve the children of the selected object without a RiuMainWindow or OpenGL context. Use it to add curated tests for the RicToggleItems feature family, toggling the show state of a well path collection's child well paths off, on and flipped.
…section tests Many command features read RiaApplication::activeReservoirView() / activeGridView() rather than the selection. Publish the mock view as the active reservoir view in RiaFeatureTestModelBuilder so these features have a valid context in the headless harness. Add curated tests for RicNewAzimuthDipIntersectionFeature and RicNewPolylineIntersectionFeature, which resolve the target view from the active view and append an intersection to it.
Add tests for RicEclipsePropertyFilterNewInViewFeature and RicNewPolygonFilter3dviewFeature, which read the active grid view and append a property filter / polygon cell filter to the view's collections.
…tests Build well paths with a real vertical trajectory and a metric unit system in RiaFeatureTestModelBuilder, so features that need a trajectory (laterals) or would otherwise prompt for a unit system (completions) run headless. Add curated tests for RicNewWellPathLateralFeature (adds a lateral well path) and RicNewPerforationIntervalFeature (adds a perforation interval to the selected well path).
…ests Move RimMockSummaryCase to a shared ApplicationLibCode/TestModels directory and put it on the include path of both ResInsight-tests and ResInsight-featuretests, so the feature tests can build in-memory summary cases without a .SMSPEC file. Add a summaryCase() builder and curated tests for RicNewDefaultSummaryPlotFeature (creates a summary multi plot from a selected case) and RicNewWellLogPlotFeature (creates a well log plot).
Add a test for RicCreatePolygonFeature, which appends a user-defined polygon to the project polygon collection when nothing is selected.
Add a CMake Formatting section describing cmake-format from the cmakelang package and the repository configuration cmake/cmake-format.py. Instruct agents to run it on changed CMake files when cmakelang is available, so the formatting fix is part of the commit instead of a follow-up bot pull request.
Add a geoMechCase() builder that loads the small VTK (.pvd) model from the unit test data via RiaApplication::openOdbCaseFromFile, giving the feature tests a GeoMech case and view without an Abaqus/ODB dependency. Use it to test RicNewCellIndexFilterFeature, which is only enabled when the active grid view belongs to a GeoMech case. Also assert the feature stays disabled for an Eclipse view.
The in-process sweep cannot be a gate: features that assume a valid GUI selection context trip CVF_ASSERT/CAF_ASSERT, which abort the process rather than throw, so the first offender ends the run and hides every feature after it. Features that block also stall the run indefinitely. Add a --feature-exec switch that executes a single feature and exits, and a parameterized sweep that re-runs this executable once per feature. A crash or hang now fails only that feature's test case and is attributed to its command id. Hangs are caught by a per-child timeout. Extract the shared model, scenario and dialog-watchdog logic into RicFeatureExecutionRunner so both sweeps use one implementation. The child closes the project before the application is destroyed, and prints a marker when feature execution is done, so a crash during harness teardown is not reported as a feature defect.
…p in the sweep Two harness defects made the subprocess sweep report crashes that were not feature defects. Calling actionTriggered() directly leaves QObject::sender() null, so every feature reading caf::CmdFeature::userData() aborted on a failed assert. Trigger the QAction instead, which is how the application invokes a feature. RiaGuiApplication::initialize() creates and shows the main window, so Qt has paint events queued for the 3D viewers. Offscreen there is no OpenGL context and caf::Viewer::paintGL() dereferences it, which was attributed to whichever feature happened to run. Drop the scheduled redraws and leave the event loop alone; rendering belongs to a separate software-GL tier. Together these take the sweep from 26 failures to 7. Also correct comments that claimed the test executable has no RiuMainWindow.
RicTogglePerspectiveViewFeature changes the projection of the active viewer and RicNewContourMapViewFeature creates and draws a new 3D view. The offscreen platform plugin creates the viewer widget but no OpenGL context, so drawing dereferences a null context in caf::Viewer::paintGL(). Both work in the real application, so they belong in a software-OpenGL test tier rather than being reported as feature defects.
The ResInsight-featuretests executable was built but never run in CI. Run it alongside the unit tests so the curated command feature tests actually gate. Also run the subprocess execution sweep, which starts one child process per registered Ric*Feature and attributes a crash or hang to a single command id. That is too slow for every push, so it is limited to the nightly schedule.
Split the model builders into helpers that do not close the project, so they can be combined, and add richModel(): an Eclipse case and view, a well path, a summary case, a GeoMech case and view, and a polygon in one project. The execution sweep needs this because a feature is only executed under a selection where it reports itself enabled, and Eclipse selections alone leave every summary, GeoMech and polygon feature untouched.
Drive the execution sweep from richModel with one scenario per domain (Eclipse case and view, well path, summary case, GeoMech case and view, polygon), keeping the active view consistent with the selection. Features actually executed rise from 223 to 244 of 470, and the wider selection immediately exposed an abort in RicDeleteItemFeature on a GeoMech view. Share allRegisteredFeatureIds from RicFeatureExecutionRunner. Both sweeps defined it in an anonymous namespace, which breaks the unity build used on Windows, where the two files end up in one translation unit. Disable the RicNewWellLogPlotFeature test. It segfaults on Ubuntu clang-19 while passing on Windows and Ubuntu gcc, and the same feature was the one that crashed in the sweep until the sweep stopped pumping the event loop.
Replace the hand picked scenarios with a walk of the project, keeping one object per class keyword. Command features dispatch on the type of the selected object, so one instance per class is enough, and it reaches the collections and definition objects most features are written against, which the hand picked list never did. Features actually executed rise from 244 to 315 of 470. Derive the active view from the candidate instead of wiring it per scenario, so selecting an object inside the GeoMech view no longer leaves the Eclipse view active. Cap executions at three per feature and log when the cap is hit, so a feature enabled for many types does not dominate the run and the truncation is visible. Ask for capabilities with capability<T>() rather than uiCapability() and xmlCapability(). Those accessors assert when the capability is missing instead of returning null, so they cannot be used to test for it, and not every object in the project has both.
Each execution now starts from an identical model. Sharing one model let a destructive feature leave debris that made a later execution fail for reasons unrelated to the object then selected, and the existing guards could not catch it because they only track the few objects they name. Such order dependent failures are false reports, and they also make a run depend on the order the candidates happen to be visited.
A failing CAF_ASSERT prints the file and line with std::printf and then calls std::abort(), which does not flush. With buffered output the parent captured everything except the message identifying where the child died.
The sweep starts one child process per registered Ric*Feature, which takes several minutes when run serially. Register it as GoogleTest shards instead, so ctest -j runs them in parallel while every feature still gets its own child process and its own test case, keeping crash isolation and attribution. Four shards complete in 103 seconds on a developer machine, and the work is evenly spread with each shard taking between 96 and 103 seconds. Note that enable_testing() is called in ApplicationLibCode, so the tests are registered in that part of the build tree and ctest has to be pointed there.
Measured on a 32 core machine: four shards 103 s, eight shards 62 s, twenty shards 61 s. Going past eight buys no wall clock time but doubles the CPU spent, because the shards become unbalanced. GoogleTest splits by test index while the cost per feature varies a lot, so one shard draws the expensive features and takes 61 s while others finish in 31 s and wait.
magnesj
force-pushed
the
investigate-ric-testing
branch
from
July 24, 2026 18:09
b567977 to
c7cf726
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on the command-feature test harness (
ResInsight-featuretests) to add asserted, model-changing tests across theRic*Featurecommand layer, plus the harness helpers needed to reach previously untestable features headlessly.Harness additions
RiaFeatureTestModelBuildernow publishes the mock view viaRiaApplication::setActiveReservoirView(). Roughly two thirds of features resolve their context fromactiveReservoirView()/activeGridView()rather than the selection, so this unlocks a large part of the command layer.RiaFeatureTestTreeView— RAII helper that registers a headlesscaf::PdmUiTreeViewthroughRiaFeatureCommandContext, letting tree-driven features (theRicToggleItems*family) resolve the children of the selected object without aRiuMainWindowor an OpenGL context.RimMockSummaryCase— moved toApplicationLibCode/TestModelsand put on the include path of bothResInsight-testsandResInsight-featuretests, so summary cases can be built in memory without a.SMSPECfile. AsummaryCase()builder is added on top of it.Curated tests
New tests covering the main selection-context groups:
The suite runs offscreen with no OpenGL and is green.
Documentation
docs/agents/coding-style.mdanddocs/agents/core.mdgain guidance to runcmake-format(fromcmakelang) with the repository config on changed CMake files when the tool is available, so formatting lands in the commit instead of a follow-up bot pull request.🤖 Generated with Claude Code