asciidoc-py2
asciidoc-py2 copied to clipboard
asciidoc-py
"Errors should never pass silently"
Errors should never pass silently - The Zen of Python
This pull request is based on the previous one #25. It improves the testasciidoc.py tool to report better the failures and the other warnings.
I recommend to review and merge #25 before considering this one.
https://github.com/florentx/asciidoc/compare/with-travis...compat26
When preparing PR #25 I discovered that the test suite is a bit too indulgent:
- if some dependency is missing, errors are printed but test is marked PASSED
- if a filter spawns a subprocess and it crashes, test is marked PASSED
- if the source of a test was deleted or a backend is missing, the test is SKIPPED but the exit code is 0
- if a macro or anything else returns a warning, the warning is silenced, and result is PASSED
This PR addresses the above issues:
- it returns exit code 1 on FAILED tests, but also on SKIPPED tests, and on warnings too
- it prints all the unexpected messages on the screen when running the test suite
- a new directive is created for
testasciidoc.confto declare the expected messages:% messages - this new directive is documented
- test is counted as WARNED if some message is unexpected, and the exit code is set to 1
I've chosen to add all the current warnings to the expected % messages, so the Travis-CI build is still green after the merge.
However, we should challenge this in the future, and probably fix some tests.
Thanks for this commit. I wonder whether we should let the Travis CI build fail instead of adding the warning messages to the list of expected ones. This will give us a reminder that we need to fix these issues.
What do you think? Michel
IMHO if we let the build red, it will not help for reviewing future PR. Instead, I propose to open a separate issue for the only real bug (from above):
The second warning is reported only with the docbook45 backend : when it is fixed, this line should be deleted.
OK. But who is going to fix it? Unfortunately, I don't know the code base good enough to do it myself.
Michel
It's not a new bug, it already lived in the Googlecode repository.
The bug is not in the core asciidoc engine: the test complains about the configuration of the "docbook45" backend specifically, which is missing a section [unfloat-blockmacro].
So it's probably only the ./docbook45.conf which needs a small fix.
Anyone interested can try it and propose a patch.
Currently I am more focused on the html output, and the core engine of asciidoc.
Because github did not refresh the PR #27 diff, here is the web view: https://github.com/florentx/asciidoc/compare/compat26
I believe that this PR is ready for final review.
docbook45issue was addressed thanks to @elextr feeback.- case of the missing file
barchart.pyis mitigated later with PR #31 where I propose to create a stub file to avoid spitting absolute path in the error message which is written to the html output.
At a quick look it seems ok, but I am currently unable to test it. I would prefer if someone (other than the originator) tested it before commit.
Otherwise I hope to get to it in the next week.
Thank you for your review.
I rebased the changes on master after PR #31 was merged, because #31 fixes the "missing barchart.py" warning.
@florentx since its a reasonably large change I'd like to test it before committing, but I'm travelling with a tablet (so no git, asciidoc etc) so all I can do is press the github merge button. If anyone else (not the submitter :) can test then I can merge.
@florentx It seems this repo will no longer accept PR as the latest release has been cut a while ago but https://github.com/asciidoc/asciidoc-py3 would. If you still want this PR integrated in asciidoc, do you mind pushing it there please? Once done I think you can close this one. Thanks!
