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

Add acceptance and unit testing #21

Merged

Conversation

mscottford
Copy link
Contributor

I have some new features that I'd like to add, and I wanted to make sure I kept all existing behavior in the process, so I added test coverage as a first step. This pull request brings the line coverage to ~85% and the branch coverage to about ~70%. Coverage is measured via simplecov integration and can be cleanly generated by running bundle exec rake.

@mscottford
Copy link
Contributor Author

In response to the DCO failure, I'll be signing all of my commits and doing a force push after that's done.

@stevespringett
Copy link
Member

Big thanks for the PR. I had to update the GitHub actions that were used. The ones specified were deprecated and no longer supported. Could you sync your fork to master? I'd like to get a passing build before merge.

@mscottford mscottford force-pushed the add-acceptance-and-unit-testing branch from 8e27d3d to 9a22bb0 Compare April 5, 2023 18:23
Signed-off-by: M. Scott Ford <[email protected]>
This better conforms to the structure that's generated by running the `bundle gem` command.

Signed-off-by: M. Scott Ford <[email protected]>
…bindir`

This better conforms to the structure that's generated when `bundle gem` is run.

Signed-off-by: M. Scott Ford <[email protected]>
`bom_builder.rb` is requiring `active_support/core_ext/hash` but `activesupport` wasn't listed in the `.gemspec` as a dependency. This likely hasn't come up before because consumers have had `activesupport` listed as dependency.

Signed-off-by: M. Scott Ford <[email protected]>
This avoids issues with `Gemfile.lock` files being used with fixtures.

Signed-off-by: M. Scott Ford <[email protected]>
This `Gemfile` and `Gemfile.lock` only references `activesupport`.

Signed-off-by: M. Scott Ford <[email protected]>
I'm not sure that acceptance tests for the short parameters makes sense. I think a unit test would be a better way to go about handling that.

Signed-off-by: M. Scott Ford <[email protected]>
Signed-off-by: M. Scott Ford <[email protected]>
This was mainly done to make sure the `simplecov` configuration was working correctly.

Signed-off-by: M. Scott Ford <[email protected]>
Signed-off-by: M. Scott Ford <[email protected]>
@mscottford mscottford force-pushed the add-acceptance-and-unit-testing branch from 9a22bb0 to aae1fec Compare April 5, 2023 18:23
@mscottford
Copy link
Contributor Author

Big thanks for the PR. I had to update the GitHub actions that were used. The ones specified were deprecated and no longer supported. Could you sync your fork to master? I'd like to get a passing build before merge.

Not a problem! I've rebased off of master and I've signed my commits.

@mscottford
Copy link
Contributor Author

I see that CI is still failing. I'm going to work on getting that to pass. I'm going to add a matrix of ruby versions to test against.

`activesupport` version 7.x does not support Ruby versions
older than 2.7.

Signed-off-by: M. Scott Ford <[email protected]>
@mscottford
Copy link
Contributor Author

I see that CI is still failing. I'm going to work on getting that to pass. I'm going to add a matrix of ruby versions to test against.

CI appears to have been failing because activesupport version 7.x does not support Ruby versions older than 2.7, so I removed Ruby 2.6 from the support matrix. If Ruby 2.6 needs to be supported, then we'll need to depend on an older version of activesupport.

Sonatype Lift is using Rubocop to report quality violations.
The default configuration Sonatype Lift is using is not
publically available. So in order to fix the comment that it
left on the pull request after the version requirement was
added to the `.gemspec` file, I added Rubocop as a development
dependency, and then generated a TODO style configuration that
excludes all of the default warnings except for the one that
Sonatype Lift reported. That one was fixed.

Signed-off-by: M. Scott Ford <[email protected]>
@mscottford
Copy link
Contributor Author

Yay! That's a passing build, @stevespringett. Please let me know if you have any code review feedback.

@stevespringett
Copy link
Member

Fantastic. Thanks so much for the PR.

@stevespringett stevespringett merged commit dd3f29e into CycloneDX:master Apr 6, 2023
@mscottford mscottford deleted the add-acceptance-and-unit-testing branch April 6, 2023 14:14
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