Add ASN tests and pry. Update .gitignore. #192
Conversation
|
Attempting to address #165 for |
cmhobbs
force-pushed
the
165-asn-tests-redux
branch
from
78e4bb5 to
94c8ed3
Compare
|
earlier tonight i noticed that i was calling the wrong methods in some of my tests due to copying and pasting a couple too many lines on little to no sleep. i think i've resolved most of those issues but it's something to look for. |
|
Well, I guess I've managed to anger CI. I'll try to fix that later. Naturally, the tests all pass on my machine 😞 ... |
|
@cmhobbs I'm going to guess the reason why the specs are failing is because they're running in a clean environment where the file hasn't even been downloaded yet, so In the stub_const("#{described_class}::List::PATH",list_file)
allow(described_class::List).to receive(:update) |
There was a problem hiding this comment.
Massive review:
- Move the example TSV data out into a
fixutres/file. - Use
stub_const("Ronin::Support::Network::ASN::List::PATH",...)instead ofoptions[:file]to force it to use the fixtures file. - For methods that print anything, we should test the
output(). - For methods that return or yield any records, we should test those as well.
| 2001:200:900::\t2001:200:9ff:ffff:ffff:ffff:ffff:ffff\t7660\tJP\tAPAN-JP Asia Pacific Advanced Network - Japan | ||
| 2001:200:a00::\t2001:200:dff:ffff:ffff:ffff:ffff:ffff\t2500\tJP\tWIDE-BB WIDE Project | ||
| TSV | ||
| end |
There was a problem hiding this comment.
Probably a good idea to move this out into a file in spec/cli/commands/fixtures/.
|
|
||
| describe Ronin::CLI::Commands::Asn do | ||
| include_examples "man_page" | ||
|
|
||
| subject { described_class.new } |
There was a problem hiding this comment.
This is actually the default value of subject.
|
|
||
| it "calls #print_asn_record" do | ||
| expect(subject).to receive(:print_asn_record) | ||
| subject.run |
There was a problem hiding this comment.
We should actually test the output of the command here. It might be OK to let it call query_ip, which will perform a DNS query. Any tests that hit the network should also be tagged with :network.
| end | ||
| end | ||
|
|
||
| # FIXME: (cmhobbs) the use of #exit has caused this to be difficult to test |
There was a problem hiding this comment.
You can do:
expect {
...
}.to raise_error(SystemExit) do |error|
expect(error.status).to eq(1)
end| describe '#download' do | ||
| context "with default parameters" do | ||
| let(:url) { 'https://iptoasn.com/data/ip2asn-combined.tsv.gz' } | ||
| let(:file) { "#{Dir.home}/.cache/ronin/ronin-support/ip2asn-combined.tsv.gz" } |
There was a problem hiding this comment.
Probably better to use Ronin::Support::Network::ASN::List::PATH here.
| 2001:200:a00::\t2001:200:dff:ffff:ffff:ffff:ffff:ffff\t2500\tJP\tWIDE-BB WIDE Project | ||
| TSV | ||
| end | ||
|
|
There was a problem hiding this comment.
Also maybe add a top-level before { stub_const("Ronin::Support::Network::ASN::List::PATH",asn_list_file) } at the top instead of setting options[:file] in all of the other specs.
| ip2asn_file.unlink | ||
| end | ||
|
|
||
| describe '#run?' do |
| end | ||
| end | ||
|
|
||
| context "when options[:update] is true" do |
There was a problem hiding this comment.
Mention "--update option" in the spec description.
| record = subject.search_asn_records.first | ||
|
|
||
| expect { subject.print_asn_record(record) }.to output( | ||
| "223.255.233.0 - 223.255.233.255 AS140694 (AU) ARAFURA-AS-AP Northern Technology Solutions\n" |
There was a problem hiding this comment.
Use #{$/} instead of \n, in case we ever want to run the specs on Windows; it's unlikely that we'll do this anytime soon, but generally is a good practice to use $/.
| end | ||
|
|
||
| it "prints the record in verbose format" do | ||
| output( |
There was a problem hiding this comment.
that got chopped out on accident when i was fixing my branch... good catch.
|
thanks for the thorough review! i'll get to work on edits soon. |
No description provided.