-
-
Notifications
You must be signed in to change notification settings - Fork 3
fix(PythonCollector): error for unexpected result #1
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
Conversation
Hi @rachmadaniHaryono, thanks for the contribution! I think there's an easier way. We see in your third error that - error = result.get("error")
- if error:
+ if "error" in result:
+ error = result["error"] |
And we should definitely add a test for it. Could you do that please? |
you mean like that? |
Exactly! That should cover all cases. |
is it enough? |
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
These tests are nice to have (we can keep them), but they don't actually test the fix here. |
i think this cover that error and traceback branching this will not test the program result is succesful |
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
i will fix error from the quality ci and find out what happen with windows ci test |
Ah, I can deal with the Windows one, just fix the quality warnings :) |
Windows runs should be fixed if you rebase on main branch. |
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
i fix everything except https://github.com/m-burst/flake8-pytest-style/blob/master/docs/rules/PT012.md i also tried this def test():
obj = collector.PythonCollector()
with mock.patch("mkdocstrings.handlers.python.collector.json.loads", return_value=retval):
with pytest.raises(CollectionError) as excinfo:
assert obj.collect("", {})
assert str(excinfo.value) == exp_res if last asert move to outer part it will raise if it kept as is it will raise PT012 i choose to ignore PT012 because it make less sense |
Oh yes, you did well, PT012 is just annoying (doesn't get |
Argh, it seems I've broken CI again. Let me fix it. |
You can rebase on main, sorry 🙇♂️ |
Oh well, the mkdocs-material dep is missing from the test group. If you don't mind, that's LGTM so I'll merge and fix CI after 🙂 Thank you very much for your contribution and prompt response to my review! |
this is fix for several error i got when trying mkdocs on https://github.com/iamtalhaasghar/yewtube
in the end i still don't know what to change when rebuild_category_lists handle empty dict
legacy error 1
legacy error 2
legacy error 3
legacy error 4