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.
2 participants
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.