Fix: Error Sidebar PR failed test #17959
Conversation
There was a problem hiding this comment.
Thanks for the pr @Dedelweiss! So just to clarify the issue was with the println? Or what was actually causing the failure? Also I see you removed -title, is that related or a separate issue?
[test_windows_full]
Dedelweiss
force-pushed
the
17229-fix-error-sidebar
branch
from
b1d304a to
020aedd
Compare
Hello Chris, thank you for your feedback. The main problem is that I didn't delete the title of the example with an error (NoTitle), so my assert was wrong for loadSidebarNoTitleError(). So I think that if there's a problem, it would probably come from there. The println is just an Int I noticed. |
|
I also added the [test_windows_full] in the body of my commit so I can test the full test run here. |
| """index: index.md | ||
| |subsection: | ||
| | - title: My title | ||
| | page: my-page1.md |
There was a problem hiding this comment.
This change makes the YAML invalid, I think there should be a - before page.
Also, I am surprised that the regression occurred only on Windows. Do you know why?
There was a problem hiding this comment.
Yes, that's the goal. This sidebar is, after that, asserted to have an error when used, with loadSidebarNoTitleError().
|
The problem is that the following line makes no sense: |
Indeed, I'm correcting that right now, thank you very much @sjrd |
There was a problem hiding this comment.
Thanks for fixing this @Dedelweiss. Since we're now removing the actual check for the page (which you added in your other PR), if you do fix this and still want to add it in a way that won't error, please do!
Sure, thank you |
The goal of this PR is to re-add the path validation of the page in a sidebar.yaml and improve the Testcase in the `SidebarParserTest` by erasing the first title, so it's easier to understand the problem in it. This PR is related to PRs: #17229 & #17959 [test_windows_full] Co-authored-by: Lucas Leblanc <[email protected]>
Backports #17959 to the LTS branch. PR submitted by the release tooling. [skip ci]
The goal of this PR is to re-add the path validation of the page in a sidebar.yaml and improve the Testcase in the `SidebarParserTest` by erasing the first title, so it's easier to understand the problem in it. This PR is related to PRs: #17229 & #17959 [test_windows_full] Co-authored-by: Lucas Leblanc <[email protected]> [Cherry-picked cf50fe3]
The goal of this PR is to re-add the path validation of the page in a sidebar.yaml and improve the Testcase in the `SidebarParserTest` by erasing the first title, so it's easier to understand the problem in it. This PR is related to PRs: #17229 & #17959 [test_windows_full] Co-authored-by: Lucas Leblanc <[email protected]> [Cherry-picked cf50fe3]
The purpose of this Pull request is to correct my PR #17229
Sorry for this failure.
I have tested the following command locally and my tests pass.
sbt ";dist/pack ;scala3-bootstrapped/compile ;scala3-bootstrapped/test"[test_windows_full]
Fixes: #17963