fixup regression regarding longtable handling (and some tests fixes) #9925
Conversation
Change happened at c8bfc13 and introduced regression by changing the condition. This commits correctly put back the right condition
…-all previously it was silently ignored, now it will error out
…ng to pdf Take the opportunity to refactor the test suite and adapt test
By throwing an uncaught error
otherwise, this is of the form pdf: null or typst: default and means only to render with default check
|
All tests are fixed now. We add some non-working tests due to wrong verify function in tests specification. This won't happen. Edit: check of feature matrix is running here: https://github.com/quarto-dev/quarto-cli/actions/runs/9423312034 |
|
Fixed some issues found in feature-format matrix docs: New run at https://github.com/quarto-dev/quarto-cli/actions/runs/9423620780 |
| - "a[href=\"#tbl-2\"].quarto-xref" | ||
| dashboard: *dom-tests | ||
| revealjs: # reveal resolves anchors differently, using the section headings instead of the float ids | ||
| ensureHtmlElements: |
There was a problem hiding this comment.
Just a head up that fixing this is breaking the feature format matrix test
https://github.com/quarto-dev/quarto-cli/actions/runs/9423620780/job/25962387025#step:26:214
So I don't know if revealjs is just not well supported here, or something else but this is probably to fix in another issue.
This PR just surfaced it as previously there was no test, and it was silently not testing
cderv
changed the title
cderv
changed the title
|
All the test fixes are great. I wonder if we should fail every test that has a |
There was a problem hiding this comment.
LGTM.
All the test fixes indicate that we need to be better at preventing bad tests from being added.
I try to explicitly ensure a test is actually failing before fixing the code, but clearly some of the tests I added would never fail because of bad syntax. So we need stronger checks.
This is what I wanted to add. I used this pattern } else {
throw new Error(`Unknown verify function used: ${key} in file ${input} for format ${format}`) ;So there will be an uncaught error I guess which will make the test fails already.... It is just an unhandled error. Live example here: |
Ah, you (of course :)) did the thing I was suggesting it would be a good idea to do! I just missed it in the PR amid the other changes. Greaat! |
This is a regression following lua regex change
Change happened at c8bfc13 and introduced regression by changing the condition.
This commits correctly put back the right condition
And also adds two tests that would have allow us to find this problem.
This PR also adds improvement in our test suites by adding a function to test the intermediate .tex file while testing the pdf output rendering with LaTeX too.
ensureLatexFileRegexis the equivalent ofensureTypstRegex.keep-tex: trueis requiredThis was done by a refactoring with a new
verifyKeepFileRegexMatchesthat could be used in our regular test suite (not smoke all). I moved function around to regroup so diff may not be easy to read. 🤔Some error are also now thrown by the test suite to help find early some problem
ensureLatexFileRegexorensureTypstRegexare used, respectivelykeep-tex: trueandkeep-typ: trueare checked as they are required.Hopefully, all the test will run ok !