Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FI-2647 Add TestKit metadata DRAFT #476

Merged
merged 3 commits into from
May 13, 2024
Merged

FI-2647 Add TestKit metadata DRAFT #476

merged 3 commits into from
May 13, 2024

Conversation

arscan
Copy link
Contributor

@arscan arscan commented Apr 5, 2024

Summary

Testing Guidance

Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 46.15385% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 80.22%. Comparing base (f8dffb1) to head (571e978).

Files Patch % Lines
lib/inferno/entities/test_kit.rb 40.00% 33 Missing ⚠️
lib/inferno/config/boot/suites.rb 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
- Coverage   80.39%   80.22%   -0.18%     
==========================================
  Files         238      240       +2     
  Lines       11821    11881      +60     
  Branches     1149     1161      +12     
==========================================
+ Hits         9504     9532      +28     
- Misses       1659     1691      +32     
  Partials      658      658              
Flag Coverage Δ
backend 93.70% <46.15%> (-0.79%) ⬇️
frontend 74.19% <ø> (ø)

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.

# description <<~DESCRIPTION
# This is a big markdown description of the test kit.
# DESCRIPTION
# suite_ids ['us_core_v311', 'us_core_v400', 'us_core_v501', 'us_core_v610']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can this be inferred based on the TestSuites defined in the same module?

A somewhat open question i think is for TestKits that are related to other test kits and it may be useful to provide direct testing for those test kits in this test kit, how do we handle that? Right now, by default we show the suites for imported test kits on the suite list page as a first-class suite, which is definitely imperfect but gives you more testing options. We are in a new paradigm here, and i'm not sure how we want to handle it. Should we just say 'nope, those are in different test kits, if you want to test those suites directly load up that test kit'. Or something more nuanced like, having the ones loaded in the same namespace be 'primary' and have 'secondary/external/additional' ones that are either explicitly listed like this?

I think we probably should just come up with an answer this week and see how it flies, instead of leaving it open-ended indefinitely.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could do that. It would require looking through all of the constants in the test kit module namespace for test suites, and then recursively looking through any submodules for test suites. In US Core, for example, the suites are all in submodules based on the US Core version. This assumes that all suites belong to a top-level module (or a submodule at some level of nesting) which the test kit will also be a member of. I think that is a fair requirement which all of our test kits probably meet, but I don't know what's out in the wild.

I don't know that it's worth it if there is a possibility that we would then have to add more methods to include/exclude particular suites, though.

# This is a big markdown description of the test kit.
# DESCRIPTION
# suite_ids ['us_core_v311', 'us_core_v400', 'us_core_v501', 'us_core_v610']
# tags ['SMART App Launch', 'US Core']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might get a little messy; i wonder if this is something that we should categorize at the platform level. But I'm good with this for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that tags might want to be defined in the platform, but I think it still makes sense to store them on the test kits. Do you want me to remove this line from the example?

# maturity 'High'
# authors ['Author One', 'Author Two']
# repo 'https://github.com/inferno-framework/us-core-test-kit'
# end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

one other idea i had was for maybe documenting extra services / tools loaded into the test kit at this level (e.g. reference-server). Which, yeah, has to be well synchronized with docker-compose and nginx etc, but i'd think is where the information should belong. Perhaps we just kick that down the road a bit, as it would involve lots more thought.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I don't think we should take that on yet.

@Jammjammjamm Jammjammjamm force-pushed the fi-2647-add-test-kit branch 4 times, most recently from 38e8c83 to 969c5ec Compare May 10, 2024 12:06
@arscan arscan marked this pull request as ready for review May 13, 2024 14:00
@arscan arscan force-pushed the fi-2647-add-test-kit branch from 969c5ec to 571e978 Compare May 13, 2024 14:01
@arscan arscan merged commit 7e7b9d5 into main May 13, 2024
9 of 10 checks passed
@arscan arscan deleted the fi-2647-add-test-kit branch May 13, 2024 14:19
@rpassas rpassas mentioned this pull request May 13, 2024
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.

2 participants