-
Notifications
You must be signed in to change notification settings - Fork 62
WIP: Test page's code blocks #74
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?
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?
pvalena
left a comment
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 '$'