-
Notifications
You must be signed in to change notification settings - Fork 8
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
License Export - Updated test scenarios #20
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Rajan Ravi <[email protected]>
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.
Nice update, covers lot of the specific cases.
One Scenario I think may be a mistake, or maybe I misunderstood what it is trying to do? (see inline)
Three suggestions for future (can be ignored for this PR, just to think about):
-
Given User is on SBOM license information file
or such generic steps cannot be used later for automation- try to think about this little bit more as code (or imagine looking at just one scenario, without the others)
- ... as it needs context information too
- ... e.g.
Given user is on SBOM license information file for sbom "cdx-single-license"
or such, - ... image that it is function call (which it actually is)
- ... and so all scenarios starting with same function call without any parameter have no way giving you the specific sample data you need to test
-
it is very helpfull to have sample data mentioned, but since they are not in public google drive disk (or at least still require google account), i think we should not be including them here
- instead we should attach sample data into this repo
- also we should use very exact crafted sample json for the testcases, avoid to have too much noise coming from the samples ... which makes it only lot harder to verify/debug
- e.g we could have few 'overall' samples (which can be used for most of the other tests where e.g. licenses does not matter) and then specific ones for 'cdx-pkg-multi-license', 'cdx-pkg-altref' and such, and as per the first point they should be named in the scenario i think (even if we do bulk upload of most of them, here it would be used for identifying them in ui/navigating to their search result line or sbom explorer page for ex.)
-
repeating all very detailed field/column assertions seems does not fit well into feature file
- i think feature file should be high level description, usually avoiding any implementation details
- so likely we should have then just single check for common parts + specific part:
And "license expression" column is matching "components.license.expression" from sbom`
- i realize without code it is better to express/record it at least here
- but having feature file full of all these copied wall of text makes it impossible to read, i have no clue if there are typos or not, my brain starts melting when i get to middle of reading the file after so many 'and license should or should not be' :) if you get what i mean
Scenario: Verify the contents on CycloneDX SBOM license reference CSV file | ||
Given User is on license reference file | ||
When User selects a license from the list of licenses | ||
Then The License reference CSV should be empty |
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 think there may be mistake here:
Why to first select license from a list ... to then assert that reference file is empty?
I think either it should be just that file is empty (as that means selecting package/license there is impossible as there are none present).
Or maybe it is not empty and in that case the other assert was to be about some column content?
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.
It is a mistake - updated the feature 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.
@queria Thanks for the feedback - I have included most of your suggestions. Please find the details below:
- Updated the steps with your suggestion like
"version" column should match "metadata.component.version" from SBOM
- Agree on the test data, I have included them as for our release testing. I have removed it.
- We may need to make a decision on the generic part. I'm somewhat inclined to use generic steps and handle data validation through code. My idea is to make the test steps more generic and parameterize the SBOM file and package information. So based on the test scenario we could pass the information.
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.
Hey @mrrajan looks good and thorough scenarios, but I have some questions inside. Looks like it covers a lot.
Signed-off-by: Rajan Ravi <[email protected]>
Added test scenarios for the license export feature