-
Notifications
You must be signed in to change notification settings - Fork 60
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
WIP: Test page's code blocks #74
base: master
Are you sure you want to change the base?
WIP: Test page's code blocks #74
Conversation
for now it tests only syntax of bash code starts with '$'
spec/code_block_spec.rb
Outdated
require 'spec_helper' | ||
require 'kramdown' | ||
require 'ap' | ||
require 'pp' |
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.
Please remove debug requirements.
spec/code_block_spec.rb
Outdated
require 'kramdown' | ||
require 'ap' | ||
require 'pp' | ||
site = File.join(File.dirname(__FILE__), '..','content', '**', '*.md') |
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 not site
, but rather content
, right?
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.
Needs refactoring still, but looks rather good.
spec/code_block_spec.rb
Outdated
def extract_code(element,prev_element) | ||
codes = [] | ||
if element.type == :codespan \ | ||
&& !element.value.nil? \ |
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.
Does this ever occur? It could probable be removed and .value
converted to empty string to return failure as the :codespan
is empty.
BTW. I quite prefer codespan
instead of codeblock
.
spec/code_block_spec.rb
Outdated
codes = [] | ||
if element.type == :codespan \ | ||
&& !element.value.nil? \ | ||
&& !codes.include?(element.value) \ |
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.
Actually, I'd prefer to include even the already existing codes and only avoid executing / checking the same one multiple times.
That's because it preserves the context
information we may need in other tests(related with the comment above).
spec/code_block_spec.rb
Outdated
if element.type == :codespan \ | ||
&& !element.value.nil? \ | ||
&& !codes.include?(element.value) \ | ||
&& ((prev_element.children.size == 1) || (element.value[/^\n/])) #element value hasn't got any sibling or begin with '\n' |
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.
Please move the comment above the if
.
spec/code_block_spec.rb
Outdated
|
||
PAGES.each do |page| | ||
generated_page = Kramdown::Document.new(File.read(page)) | ||
codes = extract_code(generated_page.root,generated_page) |
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 seems like you could pass just generated_page
instead and use .root
where needed.
spec/code_block_spec.rb
Outdated
|
||
before :each do | ||
codes.each do |code| | ||
code.gsub! "\n",'' |
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'd rather this would be solved in the extractor itself. (multiple lines = multiple commands)
spec/code_block_spec.rb
Outdated
end | ||
it 'have good syntax' do | ||
codes.each do |code| | ||
if code[/^\$/] #code starts with '$' |
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 seems to me like we want to have different tests for various code-lines
(commands).
Meaning this logic could be deciding for the test enablement, but from the level above.
spec/code_block_spec.rb
Outdated
it 'have good syntax' do | ||
codes.each do |code| | ||
if code[/^\$/] #code starts with '$' | ||
expect(system('bash', '-n', '-c', code, '&>/dev/null')).to be_truthy,"Something wrong with: '#{code}'" |
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.
We could build an Array of code-lines
+ return-codes
(per page) and run the tests afterwards.
spec/code_block_spec.rb
Outdated
codes.each do |code| | ||
if code[/^\$/] #code starts with '$' | ||
expect(system('bash', '-n', '-c', code, '&>/dev/null')).to be_truthy,"Something wrong with: '#{code}'" | ||
elsif code[/^\#/] #code starts with '#' |
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.
You're actually comparing only the first character from string, which can be done as:
code[0] == ?#
…method extract_code, add Array of results from testing bash syntax
and add comments.
Add comment, remove unnecessary parts of condition, deleting '\n' from code replace with an array of codelines without '\n' on start/end and whitespaces, rename variables, replace 'results' Array with Hash, change code structure to be easy readable
spec/code_block_spec.rb
Outdated
it 'has a good syntax' do #<<< passes syntax check | ||
codespans.each do |code| | ||
if code[/^\$/] #code starts with '$' | ||
results.push([code,system('bash', '-n', '-c', code, "&>/dev/null")]) #<<< do not execute here |
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.
Now it occured to me we could also do a lazy
evaluation. Meaning we'll define custom default for the Hash
.
rename PAGES to MD_FILES and codespans to code_lines, change extract method, add file path into decsribe error message, replace 'each' with 'inject'
Hello, do you want to finish this? :) |
for now it tests only syntax of bash code starts with '$'