-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Test coverage improvements - Core #3503
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
base: master
Are you sure you want to change the base?
Test coverage improvements - Core #3503
Conversation
| if IO.popen(%w[which espeak], 'r').read.to_s.eql?('') | ||
| print_error('[Text to Voice] eSpeak is not in $PATH (brew install espeak on macOS, apt-get install espeak on Linux)') | ||
| return | ||
| end | ||
| if IO.popen(%w[which lame], 'r').read.to_s.eql?('') | ||
| print_error('[Text to Voice] Lame is not in $PATH (apt-get install lame)') | ||
| print_error('[Text to Voice] Lame is not in $PATH (brew install lame on macOS, apt-get install lame on Linux)') | ||
| return | ||
| end | ||
| if IO.popen(%w[which espeak], 'r').read.to_s.eql?('') | ||
| print_error('[Text to Voice] eSpeak is not in $PATH (apt-get install espeak)') | ||
|
|
||
| # Load espeak gem (only if binaries are available) | ||
| begin | ||
| require 'espeak' | ||
| include ESpeak | ||
| rescue LoadError, StandardError => e | ||
| print_error("[Text to Voice] Failed to load espeak gem: #{e.message}") |
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.
Additional logging to help with new installs. This caught me out.
| RSpec.describe BeEF::Filters do | ||
| describe '.is_non_empty_string?' do | ||
| it 'nil' do | ||
| expect(BeEF::Filters.is_non_empty_string?(nil)).to be(false) | ||
| end |
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.
Replaces the old filters_spec.rb that was missing methods and combined different files. This is more specific
|
Hey @jake-the-dev , Are you still working on this PR or it's ready for review? I can see its status is Draft. Thanks |
Hi @zinduolis , I initially intended to continue this until core was sufficiently covered. However on Friday Wade had asked for this to be merged. I don't have the permissions so i assumed he was going to merge - or maybe thats something he left with you? To clarify - I will remove draft and any further iterations to core tests will be in another pr. Feel free to review if you wish, no functional code should be changed so it should all just be assertions on existing logic. |
|
Thanks, @jake-the-dev , just click Ready for review and I will review it and will let you know when you're good to merge (Wade asked me to review it). If you don't have permissions to merge, i'll happy to do that on your behalf once successful review. |
|
|
||
| describe '#oc_value' do | ||
| it 'returns option value when option exists' do | ||
| option = BeEF::Core::Models::OptionCache.create!(name: 'test_option', value: 'test_value') |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning test
option
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.
@jake-the-dev , could you please remove the 'option' line?
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 have impression that Wade would like to deprecate the beef APIs, thus API testing would be redundant. Please check with him whether that has changed. Thanks
| end | ||
| end | ||
|
|
||
| describe '.api_token' do |
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.
this is probably redundant given plan to deprecate APIs
|
Hi @jake-the-dev , I've noticed a lot of spec files are missing the Copyright statement at the top. Could you please add it? Thanks |
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.
One more API related test that will likely need to be removed given plan to deprecate APIs.
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.
One more API related test that will likely need to be removed given plan to deprecate APIs.
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.
One more API related test that will likely need to be removed given plan to deprecate APIs.
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.
One more API related test that will likely need to be removed given plan to deprecate APIs.
Pull Request
Thanks for submitting a PR! Please fill in this template where appropriate:
Category
Tests
Feature/Issue Description
Q: Please give a brief summary of your feature/fix
A: Improving test coverage utilising SimpleCov for tracking. I understand this PR is massive, but I don't think there's a quick way of introducing a large amount of coverage. This may be the least painful way.
Current coverage overview:

Current result from tests:

(note, pending tests already existed)
NOTE: the creation of tests has been AI assisted but also painstakingly reviewed and adjusted for each and every file.
Q: Give a technical rundown of what you have changed (if applicable)
A: I'm going through the beef/core files and creating tests for them ensure the core functionality is tested and trusted.
Trying not to change any business logic - only some lint issues as I come across them due to updates in Ruby over time.
Test Cases
Q: Describe your test cases, what you have covered and if there are any use cases that still need addressing.
A: Anything and everything in beef/core. Not touching
beef/extensionsorbeen/modulesin this PR as it's already large enough.Wiki Page
If you are adding a new feature that is not easily understood without context, please draft a section to be added to the Wiki below.
N/A