Instead of using a mock state, use dsstate

[kishansagathiya]

• this covers only pintracker_test.go yet
• a similar exercise would be required in stateless_test.go

[hsanjuan]

There is a big red flag here which is adding testing methods to the public interface of the package.

The diff here mixes cosmetic changes like argsCid things with things like:

		if tt.want.Status != api.TrackerStatusUnpinned {
				tt.args.st.Add(ctx, p)
			}

(can't we prepopulate with whatever needs to be in it before starting to run the testcases? There should be no logic inside the for loops other than running the method tested and check that the response matches the expectations). The TestPinTracker_Status method is worse.

Lot of things moving around and I can't compare to master, but to the pintracker PR which moved a bunch of other things around.

If we could have a mockState before, then that mockState can be turned into a pre-populated dsstate which would return exactly what the mockState would, as the responses were essentially hardcoded in the mock, and adjustments/wrappers made to handle the slow things if they can't be handled any other way (by adjusting the relevant test). The objective here is always simplify and reduce (less mocks, less special logic).

I think I prefer this to happen on the main PR and adding comments to code changes with a quick explanation on why that needs to change (or why it moved to a different place) would help me too.

[hsanjuan]

? unsure why removed

[hsanjuan]

Ideally the only thing that I should see changing in all the pintracker tests is the initialization of things plus adjustments to tests expecting different outputs from List depending on whether the pin was fast or slow. The diff should be simpler than this.

[kishansagathiya]

mockState is does not behave like a dsstate. It is returns value based on what tests need.

for example, in slow
Pins returns cid1 and cid3 with pinOpts (rmax and rmin -1), but
PinGet(cid3) would give not found (contradicting behaviour that can't be done if using dsstate)

similarlu in normal mock rpc client,
PinGet(cid2) returns pin with rmax and rmin 1
Pins returns cid2 with rmax and rmin 0

If you want to use a dsstate inside mockState, I am guessing, that would also require changing quite some tests like this.

initializing it with whatever content you need at the beginning of the tests rather than hardcoding it.

I mistook this as initializing content in the beginning of each test and not in the beginning of all tests. My bad.

I will edit this and merge this with main PR.

[hsanjuan]

mockState is does not behave like a dsstate. It is returns value based on what tests need.

for example, in slow
Pins returns cid1 and cid3 with pinOpts (rmax and rmin -1), but
PinGet(cid3) would give not found (contradicting behaviour that can't be done if using dsstate)

similarlu in normal mock rpc client,
PinGet(cid2) returns pin with rmax and rmin 1
Pins returns cid2 with rmax and rmin 0

I'm asking you to adjust tests where that special behaviour change is needed so that it is not needed anymore.

If you want to use a dsstate inside mockState, I am guessing, that would also require changing quite some tests like this.

initializing it with whatever content you need at the beginning of the tests rather than hardcoding it.

I mistook this as initializing content in the beginning of each test and not in the beginning of all tests. My bad.

I will edit this and merge this with main PR.

You might need to initialize differently on some tests, but since there was 1 mock, I would expect all test to work with a single pre-initialized state, once the things above are corrected.

[hsanjuan]

Is this PR deprecated?

[kishansagathiya]

Is this PR deprecated?

Yes, changes added in #944