WIP: Fix and add tests for website Links #73
Conversation
spec/pages_spec.rb
Outdated
| page.all(:css, 'a').each do |link| | ||
| next if link.text.nil? || link.text.empty? || !link[:href].match(/^http.*/) || urls.include?(link[:href]) | ||
| urls.push(link[:href]) | ||
| expect(url_exists? link[:href], status).to be_truthy, "expected link '#{link.text}' => '#{link[:href]}' to work (Error '#{status}')" |
There was a problem hiding this comment.
It is probably not a good idea to nest this. Move url_exists? out of the expect.
spec/pages_spec.rb
Outdated
| begin | ||
| page.find(:xpath, link.path).click | ||
| rescue NoMethodError | ||
| require 'irb';binding.irb |
There was a problem hiding this comment.
There should not be any debug code.
spec/pages_spec.rb
Outdated
| next if link.text == '' || link[:href].match(/(http|\/\/).*/) | ||
| page.find(:xpath, link.path).click | ||
| expect(page.status_code).to be(200), "expected link '#{link.text}' to work" | ||
| next if link.text.nil? || link.text.empty? || link[:href].match(/^http.*/) || link.path.nil? || urls.include?(link[:href]) |
There was a problem hiding this comment.
Can you split this into multiple lines?
Maybe add a comment?
spec/pages_spec.rb
Outdated
| @@ -4,6 +4,9 @@ | |||
| site = File.join(File.dirname(__FILE__), '..', '_site', '**', '*.html') | |||
| PAGES = Dir.glob(site).map{ |p| p.gsub(/[^_]+\/_site(.*)/, '\\1') } | |||
|
|
|||
| urls = ['http://localhost:4000', 'http://localhost'] | |||
| status='' | |||
There was a problem hiding this comment.
Isn't it possible to do with a number?
spec/pages_spec.rb
Outdated
| require 'irb';binding.irb | ||
| end | ||
| urls.push link.path | ||
| expect(page.status_code).to be(200), "expected link '#{link.text}' to work" |
There was a problem hiding this comment.
This is redundant now, right? Including the visit method.
antoninmrkvica
force-pushed
the
spec_update
branch
from
eb5f253
to
00d9e0e
Compare
antoninmrkvica
force-pushed
the
spec_update
branch
from
00d9e0e
to
3c7fafa
Compare
antoninmrkvica
force-pushed
the
spec_update
branch
from
72d5fd2
to
b7fc8bc
Compare
|
|
||
| # Array of all generated pages | ||
| site = File.join(File.dirname(__FILE__), '..', '_site', '**', '*.html') | ||
| PAGES = Dir.glob(site).map{ |p| p.gsub(/[^_]+\/_site(.*)/, '\\1') } | ||
|
|
||
| urls = ['http://localhost:4000', 'http://localhost'] |
There was a problem hiding this comment.
This can be made a Links object class's instance variable.
| links = Links.new(page.all(:css, 'a'),urls) | ||
| links.each do |link| | ||
| url=link[:href] | ||
| if !is_uri? url |
There was a problem hiding this comment.
Please use unless instead of if ! .
| url=link[:href] | ||
| if !is_uri? url | ||
| page.find(:xpath, link.path).click | ||
| expect(page.status_code).to be(200), "expected link '#{link.text}' to work" |
There was a problem hiding this comment.
In be(200) the usual practice is to replace the magic constant with a literal constant (IOW use capitalized variable defined in spec_helper.
| links.each do |link| | ||
| url=link[:href] | ||
| if is_uri? url | ||
| result = Links.url_exists? url |
There was a problem hiding this comment.
Thinking of it a bit more the method could return two values instead (you're not passing it anyway and using a global variable).
pvalena
changed the title
No description provided.