-
Notifications
You must be signed in to change notification settings - Fork 49
chore: Remove most unit tests, lint, doc testing from kokoro presubmit #1459
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
e5bea71
to
cc97b1f
Compare
bigframes/blob/_functions.py
Outdated
@@ -393,7 +393,7 @@ def pdf_extract_func(src_obj_ref_rt: str) -> str: | |||
return all_text | |||
|
|||
|
|||
pdf_extract_def = FunctionDef(pdf_extract_func, ["pypdf", "requests"]) | |||
pdf_extract_def = FunctionDef(pdf_extract_func, ["pypdf", "requests", "pypdf[crypto]"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this a merge conflict error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, merge issue, re-updated branch, shouldn't show up anymore now
"mypy", | ||
"format", | ||
"docs", | ||
"docfx", | ||
"unit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why keep unit
but remove unit_noextras
?
Edit: Oh, is it for the test coverage? Ideally we make some separate coverage targets for system vs unit tests at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, its for coverage, didn't want to mess too much with that, I doubt either unit, or integration tests individually would meet the coverage requirement. Maybe could just run unit tests as part of the integration suite though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the PR#1460 we are adding back format and mypy, but what about the others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
others are already covered by existing docs, unit test actions, so are already redundant
b/Fixes: b/401037761