Tests: Re-enable doctests, and a few cleanups #753
Conversation
The doctests require an older version, because they are missing an authority key identifier.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| Traceback (most recent call last): | ||
| ... | ||
| crate.client.exceptions.DigestNotFoundException: myfiles/041f06fd774092478d450774f5ba30c5da78acc8 | ||
| crate.client.exceptions.DigestNotFoundException: DigestNotFoundException('myfiles/041f06fd774092478d450774f5ba30c5da78acc8') |
There was a problem hiding this comment.
FYI: That adjusts for an anomaly because new code now includes the class name into the serialized string representation. I've also reflected it within a changelog item now.
| Traceback (most recent call last): | ||
| ... | ||
| crate.client.exceptions.BlobLocationNotFoundException: locations/040f06fd774092478d450774f5ba30c5da78acc8 | ||
| crate.client.exceptions.BlobLocationNotFoundException: BlobLocationNotFoundException('locations/040f06fd774092478d450774f5ba30c5da78acc8') |
|
|
||
| def __str__(self): | ||
| return f"{self.__class__.__qualname__}('{self.table}/{self.digest})'" | ||
| return f"{self.__class__.__qualname__}('{self.table}/{self.digest}')" |
There was a problem hiding this comment.
FYI: That just fixes a little formatting typo discovered when re-enabling doctests.
| # Unit tests. | ||
| suite.addTest(makeSuite(CursorTest)) | ||
| suite.addTest(makeSuite(HttpClientTest)) | ||
| suite.addTest(makeSuite(KeepAliveClientTest)) | ||
| suite.addTest(makeSuite(ThreadSafeHttpClientTest)) | ||
| suite.addTest(makeSuite(ParamsTest)) | ||
| suite.addTest(makeSuite(ConnectionTest)) | ||
| suite.addTest(makeSuite(RetryOnTimeoutServerTest)) | ||
| suite.addTest(makeSuite(RequestsCaBundleTest)) | ||
| suite.addTest(makeSuite(TestUsernameSentAsHeader)) | ||
| suite.addTest(makeSuite(TestCrateJsonEncoder)) | ||
| suite.addTest(makeSuite(TestDefaultSchemaHeader)) | ||
| suite.addTest(doctest.DocTestSuite("crate.client.connection")) | ||
| suite.addTest(doctest.DocTestSuite("crate.client.http")) |
There was a problem hiding this comment.
FYI: The omitted suites are covered by regular pytest test cases now. Thank you @surister!
Before:
myfiles/041f06fd774092478d450774f5ba30c5da78acc8
After:
DigestNotFoundException('myfiles/041f06fd774092478d450774f5ba30c5da78acc8')
This applies to both `DigestNotFoundException` and
`BlobLocationNotFoundException`.
It's usually nothing the end user is concerned with. However, on a stable project, it certainly should be mentioned to indicate potential flaws when packaging a project, but let's spare the details.
This is a library and not an application.
amotl
force-pushed
the
re-enable-doctests
branch
from
0e517d4 to
20bd23d
Compare
| "ruff<0.15", | ||
| "setuptools>=80.9.0", | ||
| "stopit<1.2", | ||
| "urllib3<2.4", |
There was a problem hiding this comment.
FYI: This can be removed after tackling #708.
| "urllib3<2.4", |
There was a problem hiding this comment.
This patch will resolve it.
Problem
It looks like recent refactorings lost the doctests?
What else?
Other than the fix for the problem at hand, the PR also includes a few more other cleanups.