Skip to content
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

Pull out relevant tests from interface-ipfs-core #5

Closed
1 of 2 tasks
SgtPooki opened this issue Aug 23, 2022 · 12 comments · Fixed by #83
Closed
1 of 2 tasks

Pull out relevant tests from interface-ipfs-core #5

SgtPooki opened this issue Aug 23, 2022 · 12 comments · Fixed by #83
Assignees
Labels
dif/medium Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP status/in-progress In progress

Comments

@SgtPooki
Copy link
Member

SgtPooki commented Aug 23, 2022

Originally from ipfs/kubo#9125

Not an action item I feel comfortable deciding upon, so I'll leave this for @aschmahmann and @lidel for now.

Kubo RPC Client should have its own tests, stripped out from interface-ipfs-core. And we should only bring over the tests that are applicable for Kubo's supported RPC calls. -- ipfs/kubo#9125 (comment)

@SgtPooki SgtPooki changed the title decide if js-kubo-rpc-client CI should reuse tests against Core APIs interface-ipfs-core Pull out relevant tests from interface-ipfs-core Aug 25, 2022
@SgtPooki
Copy link
Member Author

@achingbrain @lidel https://github.com/ipfs/js-ipfs/blob/master/packages/interface-ipfs-core/test/interface.spec.js seems to be empty (had it's only line, use strict, removed by ipfs/js-ipfs#3879).

If there are no tests in that package, where should I look for moving old ipfs-http-client tests to this repo?

@SgtPooki SgtPooki added kind/enhancement A net-new feature or improvement to an existing feature need/community-input Needs input from the wider community status/blocked Unable to be worked further until needs are met P1 High: Likely tackled by core team if no one steps up labels Sep 28, 2022
@SgtPooki SgtPooki self-assigned this Sep 28, 2022
@BigLep
Copy link
Contributor

BigLep commented Oct 6, 2022

@SgtPooki : do you have clarity now on where the relevant tests are?

@SgtPooki SgtPooki added status/ready Ready to be worked and removed status/blocked Unable to be worked further until needs are met labels Oct 10, 2022
@SgtPooki
Copy link
Member Author

@BigLep not yet

@achingbrain
Copy link
Member

achingbrain commented Oct 10, 2022

When running tests using ipfs-http-client against go-ipfs the tests are loaded from https://github.com/ipfs/js-ipfs/blob/master/packages/ipfs/test/interface-http-go.js

The complete test suite is imported:

import * as tests from 'interface-ipfs-core'

each member of tests is part of the test suite, you either run all the tests in that suite:

// test ipfs.repo.* commands
tests.repo(commonFactory)

..or you skip some of the tests in the suite because there's been a divergence in the implementations:

// test root commands, e.g. ipfs.add, ipfs.cat, etc
tests.root(commonFactory, {
    skip: [
      {
        name: 'should add with mode as string',
        reason: 'TODO not implemented in go-ipfs yet'
      },
      {
        name: 'should add with mode as number',
        reason: 'TODO not implemented in go-ipfs yet'
      },
      //... etc

Hopefully that helps.

@SgtPooki
Copy link
Member Author

Thanks that helps.

@SgtPooki SgtPooki added P0 Critical: Tackled by core team ASAP need/analysis Needs further analysis before proceeding status/in-progress In progress dif/medium Prior experience is likely helpful and removed P1 High: Likely tackled by core team if no one steps up need/community-input Needs input from the wider community status/ready Ready to be worked need/analysis Needs further analysis before proceeding labels Oct 11, 2022
@BigLep
Copy link
Contributor

BigLep commented Oct 17, 2022

@SgtPooki : My general perspective is we should make things no worse. Whatever was skipped previously can stay skipped. (I don't expect any disagreement there.)

It looks like though that you have some tests that were passing in interface-ipfs-core but are not passing in js-kubo-rpc-client? Is that right? Do we have any insight into why that is occurring?

@SgtPooki
Copy link
Member Author

@BigLep Most of the reasons these are failing are due to dependency variations between the ipfs (js-ipfs) and kubo-rpc-client packages. I don't want to synchronize the dependencies, because that's one of the problems we're trying to escape from.

Others are less obvious:

  1. Are some tests depending on proc functionality that doesn't exist in ipfs-http-client/kubo-rpc-client? 49fd3c0
  2. Are newer/changed dependencies in kubo-rpc-client, that may be locked in sub-packages of ipfs, causing failures like with 32ebdee ?
  3. Is it the upgrade to a newer ipfsd-ctl that wasn't done in ipfs-http-client prior to porting code into kubo-rpc-client like with 05346a1, where the .id() returns an instance of a PeerID instead of a string?

I'm discovering more as I go, but all the changes I've made thus far are at master...52-remove-tests-that-are-skipped

My current plan is to continue what I've been doing: fixing simple/obvious/trivial failures, and spend no more than 30 minutes on failures I can't resolve myself.

There are 708 tests ran in node environment, 705 ran in browser environment, and 705 ran in webworker environment. Spending too much time on any of these feels like the wrong thing.

@SgtPooki
Copy link
Member Author

Also, there may be a number of tests that could* have been enabled months/years ago that weren't: 565ed08

I'm also testing those when doing so isn't time-prohibitive.

@SgtPooki
Copy link
Member Author

SgtPooki commented Oct 17, 2022

one particularly troublesome library that i'm now officially marking as broken during transition has to do with using the iso-random-stream library, where we use randomBytes to write file content, and then try to read that content from ipfs, and ensure that they're equal. The way the tests are currently written, many of these (4 so far) are failing with things like expected Buffer[ 235, 46, 147, 191, 105, …(308) ] to deeply equal Readable{ …(6), …(1) }


1) kubo-rpc-client tests against Kubo
       .files.cp
         copies a file to new location:
     AssertionError: expected Buffer[ 105, 130, 20, 193, 1, 6, …(389) ] to deeply equal Readable{ …(6), …(1) }
      at Context.<anonymous> (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/js-kubo-rpc-client/test/interface-tests/src/files/cp.js:144:29)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  2) kubo-rpc-client tests against Kubo
       .files.mv
         moves a file:
     AssertionError: expected Buffer[ 235, 46, 147, 191, 105, …(308) ] to deeply equal Readable{ …(6), …(1) }
      at Context.<anonymous> (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/js-kubo-rpc-client/test/interface-tests/src/files/mv.js:59:29)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  3) kubo-rpc-client tests against Kubo
       .files.read
         reads a small file:
     AssertionError: expected Buffer[ 233, 149, 147, 85, 19, …(-12) ] to deeply equal Readable{ …(6), …(1) }
      at Context.<anonymous> (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/js-kubo-rpc-client/test/interface-tests/src/files/read.js:44:29)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      
  30) kubo-rpc-client tests against Kubo
       .files.write
         rewrites really big files:
     AssertionError: expected Buffer[ 11, 140, 199, 227, 94, …(307071) ] to deeply equal Readable{ …(6), …(1) }
      at Context.<anonymous> (file:///Users/sgtpooki/code/work/protocol.ai/ipfs/js-kubo-rpc-client/test/interface-tests/src/files/write.js:492:35)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

I imagine @achingbrain or @hugomrdias could probably provide insight on these, but I'm moving on for now.

@SgtPooki
Copy link
Member Author

SgtPooki commented Oct 18, 2022

per discussion in kubo standup today: I'm trying to ensure deps are synced in kubo-rpc-client to the deps resolved when running interface-http-go tests from js-ipfs/packages/ipfs,

  • need to manually resolve dependencies between js-ipfs/packages/ipfs, /packages/interface-ipfs-core, and /packages/ipfs-http-client to the dependencies that existed when kubo-rpc-client was pulled out of ipfs-http-client
  • Last commit to ipfs-http-client prior to js-kubo-rpc-client was Aug 17

@SgtPooki
Copy link
Member Author

Using https://github.com/ipfs/js-ipfs/releases/tag/ipfs-v0.64.2 npm install isn't working on ipfs/js-ipfs@39dbf70

@SgtPooki SgtPooki linked a pull request Dec 5, 2022 that will close this issue
8 tasks
@SgtPooki
Copy link
Member Author

SgtPooki commented Dec 5, 2022

Update of status provided in original tracking issue for kubo rpc client work: ipfs/kubo#9125 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dif/medium Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP status/in-progress In progress
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants