aleph-im / aleph-client

Lightweight Python Client library for the Aleph.im network
MIT License
11 stars 12 forks source link

Improving unit tests for Aleph client commands #197

Open FunttasticLabs opened 5 months ago

FunttasticLabs commented 5 months ago

Besides giving the Aleph CLI a full suite of integration tests with real API calls, we fixed several bugs:

Test suite might sometimes be flaky due to HTTP 502 on the API side. We might need some retry mechanism in either the SDK or the CLI itself.

hoh commented 4 months ago

Hey, what's the status of this PR ? How much work before something can be merged ?

danilo-silva-funttastic commented 4 months ago

Hey, what's the status of this PR ? How much work before something can be merged ?

Hey @hoh,

for now, please consider it as a draft, we still need to allocate more hours and finish them all.

We are going command by command and trying to make each of the tests pass.

We expect to be doing that in the next coming days.

I'll keep you posted! :)

MHHukiewitz commented 4 months ago

Some remarks on the failing tests:

We don't need exact pattern matching for the expected output. Things like the node list might change in future. We only expect these commands to return a successful exit code (0).

hoh commented 3 months ago

Hello,

This merge request looks nice at first sight, but it has been open for more than 2 months.

I see that tests still fail. Can we get something merge-able and keep future work on a subsequent merge request ?

MHHukiewitz commented 3 months ago

Yup, I'll take that over as Danilo is now busy with a different project our team is working on

FunttasticLabs commented 3 months ago

Hey @hoh,

Really sorry about that... it's not because of laziness or lack of commitment, it's just because I've been really really busy and I couldn't move forward.

In any case, we've advanced a lot with the tests but several other ones needs fixing.

I've also introduced some "core" changes that I'd like to discuss with you guys in the end, since some methods seems to have some little bugs (at least in terms of the return). I'm going to refer you there so you could take a look later.

I'm going to talk with @MHHukiewitz about a suggestion that we could use to move forward with the tests here.

danilo-silva-funttastic commented 3 months ago

Disclaimer: I commented with the company's account by mistake ^^

MHHukiewitz commented 2 months ago

@aliel I'm not sure about integration tests for the DNS service. Anything I should be careful about?