-
Notifications
You must be signed in to change notification settings - Fork 5
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-2748: Add TestKit to template #561
Conversation
require_relative 'version' | ||
|
||
module <%= module_name %> | ||
class Metadata < Inferno::TestKit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not wild about Metadata
as the class name. Maybe TestKitMetadata
would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the Metadata
, it makes the global name SomethingTestKit::Metadata
which makes sense since the RubyGem itself is the Test Kit, SomethingTestKit
is the namespace, and Metadata
is really just metadata and maybe config.
If anything I would rename/alias Inferno::TestKit
to Inferno::TestKitMetadata
.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #561 +/- ##
==========================================
+ Coverage 84.37% 84.44% +0.06%
==========================================
Files 265 265
Lines 11475 11487 +12
Branches 1261 1265 +4
==========================================
+ Hits 9682 9700 +18
+ Misses 1783 1777 -6
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
# tags ['SMART App Launch', 'US Core'] | ||
# last_updated '2024-03-07' | ||
version VERSION | ||
maturity 'Low' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should force a standard here. I'm ok not for now though i suppose just to get through this first iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to make the line in Gemfile gem 'inferno_core', path: '../'
to make bundle exec inferno start
work from within my_test_kit. After that everything in Testing Guidance is working as expected. I'm just waiting for feedback on the comments to approve.
require_relative 'version' | ||
|
||
module <%= module_name %> | ||
class Metadata < Inferno::TestKit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move the class Metadata
(or whatever we call it) into lib/%library_name%.rb.tt (i.e: lib/my_test_kit.rb)? And then the content currently in lib/my_test_kit.rb should be moved into lib/my_test_kit/my_test_kit_suite.rb. Then people can copy-paste the suite file to kick start Test Kits that house multiple Test Suites like PDex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think we should move the current contents into a separate suite file, but I don't think we should move the metadata there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so lib/my_test_kit.rb
will only have a couple of includes in it?
I don't care that much one way or another, but Shaumik's suggestion sounds pretty reasonable to me. This way lib/my_test_kit.rb
contains metadata about the test kit in a very obvious file (the 'most important' file), and we have fewer files to deal with in the main gem directory? I can see if you wanted to make it so you can load just the Test Kit metadata without loading everything "within" the test kit too. But is there a use case for that? Unfortunately we can't reference this thing in our .gemspec, which means it can't be used like version.rb is used and why its great to have that in its own file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, my goal was to be able to load just the test kits without loading the suites, but I guess the platform will need the suites anyway.
@@ -0,0 +1,3 @@ | |||
module <%= module_name %> | |||
VERSION = '0.0.1'.freeze |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea: default to 0.0.0
, and then semantically we consider a version of 0.0.0
effectively 'unversioned' (as it was before) and hide it in the UI, etc. When I first start working on something I kinda hate seeing that 0.0.1 as I consider it 'versionless' until I assign it a version.
Another nice thing is that when the person is prepping for a first release they need to bump this version, which trains them to update a version just prior to release. And having it be 0.0.0
gives them a hint as far as what the expected version setup should be (vs. an empty string).
Or we could even do 0.0.0-dev
if we wanted to lean into that practice more, but I think even on (g)(10) we found doing that to be more work than necessary (and this use of 'dev' is a little unusual because its not really a 'prerelease' as is supposed to go after "-").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like semver 2.0 recommends using 0.1.0
for an initial development release, so I'd argue we start with that (for whatever reason it looks way less jarring to me than 0.0.1
too), or go with 0.0.0
as an 'pre-versioned' version that gets hidden in the UI as its just clutter to start, and should be updated as part of the normal first release cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like 0.0.0
.
c6df251
to
b717e19
Compare
b717e19
to
9f9e71f
Compare
9f9e71f
to
cd5e7a1
Compare
Summary
This branch adds a
TestKit
by default to test kits created byinferno new
. By default, suites will now take their versions from their test kits.Testing Guidance
Run
bundle exec bin/inferno new my-test-kit
. Then you need to point the new test kit at this branch. So in the new test kit:inferno_core
requirement in the gemspecbundle install
Then you can test out the functionality in the new test kit
bundle exec inferno c
. Then in the console,Inferno::Repositories::TestKits.new.all
should include the test kit.bundle exec inferno start
. You should see the suite version in the UI, which is now coming from the test kit.