FI-3600: Requirements Support #588

vanessuniq · 2025-01-14T02:56:19Z

Summary

This branch introduces support for managing and accessing Requirements. The following changes were made:

Created an entity and repository for requirements.
Enabled automatic loading of requirements at boot time, similar to presets.
Exposed requirements through the JSON API.
Added sample requirements to the dev suite to support UI implementation.

To ensure requirements are automatically loaded from gems, their gemspecs must be updated to:

Include the CSV files. (if not included already. Or may use git as done in the template)
Set spec.metadata['inferno_test_kit'] = 'true'.

Testing Guidance

Add the following to the Gemfile:

gem 'onc_certification_g10_test_kit',
  git: 'https://github.com/onc-healthit/onc-certification-g10-test-kit.git',
  branch: 'fi-3600-demo'

Then follow the remaining instructions for running g10 from core in the Gemfile.
Run the server: bundle exec puma
Requirements should be exposed at http://localhost:4567/inferno/api/requirements: you should see both requirements from the dev_suite (id: sample-criteria-proposal) and onc_certification_g10_test_kit (onc-certification-g(10)).
Retrieve Single requirement: http://localhost:4567/inferno/api/requirements/{REQUIREMENR_ID}

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

codecov · 2025-01-14T02:57:59Z

Codecov Report

Attention: Patch coverage is 97.94521% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.45%. Comparing base (e3f14cf) to head (8d00643).

Files with missing lines	Patch %	Lines
lib/inferno/config/boot/requirements.rb	89.47%	2 Missing ⚠️
lib/inferno/dsl/runnable.rb	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
+ Coverage   84.28%   84.45%   +0.16%     
==========================================
  Files         274      281       +7     
  Lines       11601    11747     +146     
  Branches     1291     1307      +16     
==========================================
+ Hits         9778     9921     +143     
- Misses       1815     1818       +3     
  Partials        8        8

Flag	Coverage Δ
backend	`92.79% <97.94%> (+0.16%)`	⬆️
frontend	`79.11% <ø> (ø)`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

karlnaden · 2025-01-14T03:25:01Z

lib/inferno/config/boot/requirements.rb

+        .specs
+        .select { |spec| spec.metadata.fetch('inferno_test_kit', 'false').casecmp? 'true' }
+
+    files_to_load = Dir.glob(['lib/*test_kit/requirements/*.csv'])


This will pull in the out of scope requirements files as well, which shouldn't be loaded in the same way - see https://github.com/inferno-framework/subscriptions-test-kit/tree/main/lib/subscriptions_test_kit/requirements for example.

Alright, should we pull in the out-of-scope requirements or exclude them for now? If we include, I can update the Requirement model to add the extra fields (reason and details) along with a boolean field or method like in_scope?. Let me know if that makes sense.

The out of scope requirements are a subset of the requirements that will not be tested. The idea is that putting requirements into that file is the way of saying they shouldn't be expected to be verified by any tests.

Assuming that we want this requirements API to represent the requirements coverage matrix at the right side of that diagram, then I think you're right that we want to add fields to support the reason and details, or rather out_of_scope_reason and out_of_scope_details or something similar tying it to the out-of-scope purpose and source). That said, right now you don't appear to be including links from requirements to tests in the requirements repo and these files are really meant to be partners of the verifies_requirements annotations. Personally, I think it would make sense to incorporate into the requirements database the logic that determines requirement coverage and allow it to be exported to the requirements_coverage csv file. If you're not going to do that (or at least not in this PR), then I would skip processing these out of scope files.

Which requirements go with which tests is stored on the tests, not on the requirements.

Understood. I think that means that you don't want to include details from the out of scope files in the requirements repository and that those files should be ignored during this load.

Jammjammjamm · 2025-01-14T13:44:22Z

lib/inferno/repositories/requirements.rb

+          combined_id = "#{req_set}@#{id}"
+
+          # Processing sub requirements: e.g. "170.315(g)(31)_hti-2-proposal@5,17,23,26,27,32,35,38-41"
+          sub_requirements = if sub_requirements_field.nil? || sub_requirements_field.strip.blank?


You don't need to strip because blank? already accounts for whitespace.

Jammjammjamm · 2025-01-14T13:47:11Z

dev_suites/dev_requirements/requirements_suite.rb

+    id :ig_requirements
+    description 'Suite Description'
+
+    verifies_requirements 'sample-criteria-proposal@1', 'sample-criteria-proposal@2',


Shouldn't higher-level groups automatically inherit all of the requirements their children verify?

yeah, makes sense.

I don't think they should automatically inherit. Example illustrating why:

Group 1

Test A -> requirement 1

Test B (optional) -> requirement 2

An execution of Group 1 where Test A succeeds and Test B fails would result in Group 1 succeeding but not in requirement 2 being met. I think we want to keep the property that if a runnable succeeds then that implies that all its associated requirements are met. Inheritance would break this property because Group 1 succeeded but requirement 2 is not met, so Group 1 cannot inherit the requirements from its children.

Its possible that I'm wrong about wanting a success to indicate requirements are met, but my gut says that's important, so if you have different thoughts, I'd want to talk through it further.

Jammjammjamm · 2025-01-14T13:47:17Z

lib/inferno/apps/web/router.rb

@@ -45,6 +45,11 @@ module Web
              as: :check_configuration
        end

+        scope 'requirements' do
+          get '/', to: Inferno::Web::Controllers::Requirements::Index, as: :index


This seems like a problem. I don't think there is a use case for loading all of the requirements for every test kit, so we need some way to scope them down to only the requirements relevant for a particular suite.

You're thinking in a deployment where we have lots of test kits running / available within a single deployment?

The vision I had was for the -requirements.csv file to have all requirements relevant to the test kit. Then suites within the test kit are associated with a specific set of requirements via actors - each suite is associated with an actor and so requirements for that actor from the test kit's requirement set apply to that suite. Actor mapping through referenced IGs can be a bit tricky and thinking about this, I think the tools I created are doing it wrong by mapping during the creation of the set of test kit requirements, probably should be after the fact so that requirements, e.g., from the HRex IG that are shared by multiple test kits and suites have the same representation in this database when it includes requirements from those different kits / suites.

If I'm understanding Steve's suggestion correctly, it is that requirements in the database need to be associated with a test kit and suite in the database so that filtering can happen. Association with the test kit is from the -requirements.csv file. Association with each suite is designed to be based on the actor with maps from suites to actors present in a requirements configuration file that looks something like this:

test_kit_id: example_test_kit suites: - id: suite_a class_name: ExampleTestKit::SuiteA suite_actor: Provider requirement_sets: - id: set-i actor_map: - {spec: Source, test_kit: Provider} - id: set-ii actor_map: - {spec: EHR, test_kit: Provider}

Open to suggestions on how to tweak this, but I think the basic principles are there. Again, not sure if this is in scope for this PR or not.

You're thinking in a deployment where we have lots of test kits running / available within a single deployment?

Yes.

If groups inherit all of their child's requirements, then it should be easy to get the all of relevant requirements for a particular suite (which I think is the use case we actually care about), but the routing will have to reflect that rather than being a plain /requirements route.

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

…te for a given session Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

Jammjammjamm · 2025-01-16T13:48:52Z

lib/inferno/entities/test_suite.rb

@@ -108,6 +108,11 @@ def configuration_messages(new_messages = nil, force_recheck: false)
            end
        end

+        # @private
+        def suite_requirements(suite_options)
+          children(suite_options).flat_map(&:verifies_requirements)


children is only direct children, not all descendents, so we'll need a recursive method to traverse them all.

Jammjammjamm · 2025-01-16T13:50:25Z

docs/swagger.yml

@@ -61,6 +61,33 @@ paths:
            $ref: "#/definitions/TestSuite"
        "404":
          description: "Test suite not found"
+  /test_suites/{test_suite_id}/sessions/{session_id}/requirements:


Let's do /test_suites/{test_suite_id}/requirements/ with session_id as an optional query parameter.

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

vanessuniq added 3 commits January 13, 2025 10:56

Added support for requirements

566abbc

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

added dev suite for UI implementation support

c10ff1f

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

expose requirements via json api

b188c5d

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

vanessuniq requested a review from Jammjammjamm January 14, 2025 02:56

Merge branch 'main' into fi-3600-requirement-support

d9f6af5

karlnaden self-requested a review January 14, 2025 03:18

karlnaden reviewed Jan 14, 2025

View reviewed changes

Jammjammjamm reviewed Jan 14, 2025

View reviewed changes

vanessuniq added 11 commits January 14, 2025 12:50

refactor

e4d8ec0

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

exclude out of scope files

f7e65d3

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

typo

33e101b

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

verifies_requirements spec

837fa60

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

update sample requirements to add url

42d1c27

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

update dev suite

8a90054

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

scope requirement endpoint to the suite to get requirements for a sui…

1112fa9

…te for a given session Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

update swagger

e25cda3

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

Merge branch 'main' into fi-3600-requirement-support

68f4007

install dependencies

602d7e3

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

lint

72e2b45

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

vanessuniq requested review from Jammjammjamm and karlnaden January 16, 2025 00:10

Jammjammjamm reviewed Jan 16, 2025

View reviewed changes

vanessuniq added 4 commits January 16, 2025 15:59

update to collect requirements of all descendants of a runnable

6125d32

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

update requirements endpoint

aa0214a

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

update swagger

ba03aad

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>

Merge branch 'main' into fi-3600-requirement-support

8d00643

vanessuniq requested a review from Jammjammjamm January 21, 2025 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FI-3600: Requirements Support #588

FI-3600: Requirements Support #588

vanessuniq commented Jan 14, 2025

codecov bot commented Jan 14, 2025 •

edited

Loading

karlnaden Jan 14, 2025

vanessuniq Jan 14, 2025 •

edited

Loading

karlnaden Jan 15, 2025 •

edited

Loading

Jammjammjamm Jan 15, 2025

karlnaden Jan 15, 2025

Jammjammjamm Jan 14, 2025

Jammjammjamm Jan 14, 2025

vanessuniq Jan 14, 2025

karlnaden Jan 15, 2025

Jammjammjamm Jan 14, 2025

karlnaden Jan 15, 2025 •

edited

Loading

Jammjammjamm Jan 15, 2025

Jammjammjamm Jan 16, 2025

Jammjammjamm Jan 16, 2025

FI-3600: Requirements Support #588

Are you sure you want to change the base?

FI-3600: Requirements Support #588

Conversation

vanessuniq commented Jan 14, 2025

Summary

Testing Guidance

codecov bot commented Jan 14, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

vanessuniq Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

karlnaden Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlnaden Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 14, 2025 •

edited

Loading

vanessuniq Jan 14, 2025 •

edited

Loading

karlnaden Jan 15, 2025 •

edited

Loading

karlnaden Jan 15, 2025 •

edited

Loading