WIP: Test page's code blocks#74
WIP: Test page's code blocks#74antoninmrkvica wants to merge 5 commits intodeveloper-portal:masterfrom
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.
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.
This is not site, but rather content, right?
pvalena
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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 '$'