Git Product home page Git Product logo

Comments (10)

galargh avatar galargh commented on June 2, 2024

Thanks for the input! All good points 🤝 I'm happy to engage more once I'm back; I just wanted to quickly share a few thoughts.

I think declarative tests definition is what could really make the setup future-proof. It makes every other component - testing library, reporters, HTTP library, etc. - easily replaceable. Even replacing the language should be pretty straightforward.

I think that as long as we do keep these components easily replaceable, it's OK to lean on libraries that help us go faster. Especially now, in these early stages, where we want to verify if the approach even works for what we're trying to achieve.

from gateway-conformance.

SgtPooki avatar SgtPooki commented on June 2, 2024

Unfortunately, this rant of mine probably isn't as helpful as it could be.. but I'm going to "send it" anyway.

tbh: a lot of the reason the ESM migration was painful was because of obfuscator "cheatsheet" packages like create react app (CRA). This bundles up a ton of config, obfuscating a lot of what's happening, trying to do magic for you, and not allowing configuration when more advanced things are needed. My advice here for anyone is to never let CRA sit in your package for more than a week. If you're getting feature requests, EJECT first. It's great for starting up, but a horrible horrible mess for anything of significance. Maybe that's a hot-take, but I've been familiar with getting into the weeds of webpack for a long time.

As far as all the dependencies and the pain with webui, ipld-explorer-components, and all the js-ipfs stuff (even current libp2p, multiformats, etc..).. part of the problem there is that we wrote components which are supposed to be consumable/pluggable, but enforce their own dependencies. It's a plugin/module system where you can only plugin when all dependencies match, or are forced to override. So basically, we split up our packages into a lot of little packages that all have the same problem: explicit reliance on specific dependencies, and implicit reliance on functionalities of the collections of those dependencies.

We should use a repository, or other abstraction layer, where the interface is what's agreed upon, and the details and implementations are interchangeable. I don't see that we've done that for a lot of our packages, but reasonably so in the JS landscape of old.

Whether we choose the JS world or not, JS itself should not be allowed directly for anything we expect to live longer than a few months: TypeScript only moving forward. With abstraction layers, generics, and dependency injection of "loosely defined" interfaces. Define your consumptions as a relationship, and within that relationship, the provider and receiver, and finally the message structure, before you start using the code.


One example i'm seeing now (trying to get kubo-rpc-client into webui):

we're using ipld-explorer-components, which is it's own page (you two know) but that page accepts only one non-primitive input (1 callback, then 1 bool and 2 strings)

    <ExplorePage
      runTour={toursEnabled}
      joyrideCallback={handleJoyrideCallback}
      gatewayUrl={availableGatewayUrl}
      publicGateway={publicGateway}
    />

This package has dependencies that it's trying to load up, that are no longer in sync with what webui has in my branch, and it's causing issues. That component should not have runtime dependencies on any of the following:

https://github.com/ipfs/ipld-explorer-components/blob/23cf07da8a78b1d6d9aaa20a8e5a32ef42936ad8/package.json#L127-L135

and should instead accept an object of a particular interface that has the methods it needs to call to do its work and returns the data in the expected format. If all of our dependencies accepted an injected IPFS provider, consumers could override anything they need to match the latest and greatest or flex where they need to adjust for particular business needs. It would also remove the need for each of our packages to have different build artifacts for various platforms, only some object that looks like { neededMethod1, neededMethod2, neededMethod3 } and has that explicitly defined.

Inversion of Control(IOC) would help a lot with our dependency hell, because consumers would always have the right of way, instead of each package trying to enforce it.

from gateway-conformance.

laurentsenta avatar laurentsenta commented on June 2, 2024

Thanks for raising these points, I share the js-ecosystem frustration as well @SgtPooki.

I don't know the problem space enough to push for any option, but I would favor anything that lowers friction with eventual maintainers.

A few questions about constraints/expectations:

  • Is it correct to assume the tests themselves are going to be quite simple ("send request, check bytes") and that the complex part (which requires libraries, I/Os, etc) will be fixture generation?
  • If we have to choose between the ease of maintenance versus a low barrier to entry for new test writers, which should we pick?
  • Is it possible to write a data-driven / declarative test suite for the gateway specs?

from gateway-conformance.

darobin avatar darobin commented on June 2, 2024

Just some quick notes which I hope help.

  • Dependency hell is never nice, no argument there. In the early days of WPT the primary issues I felt were with Python (being forever stuck between 2.x and 3). I think that this is just a matter of knowing what you're relying on. In WPT the testharness.js is home grown and to my knowledge that was never a problem, it's vanilla and just works. Relying on something like Mocha is 100% safe IMHO, I have tests I wrote under Mocha almost ten years ago and they still run.
  • I would say unless you really have to, do not rely on React or in general anything fashionable. JS and npm are perfectly fine if you stay away from the cool kids IMHO. As @SgtPooki says, certainly do not rely on CRA. (They forgot a letter in the acronym.) I might be missing something but I don't think that anything complicated should be required here? Between declarative tests and the fact that this is mostly "make a request, see if it responds right" I don't see the need for any complex UI work? For anything that needs to render in a browser, I can't imagine it being complex enough to require anything beyond plain old DOM and maybe a couple web components to provide some encapsulation? That kind of stuff will still run after we're all dead.
  • To @laurentsenta's question, my instinct is to focus on lowering the barrier to entry for test writers, but test writers need to be able to just clone a repo, install deps, and things should just work — otherwise they can't try out their tests. So these two aren't fully in tension; dependency hell hurts both!

from gateway-conformance.

lidel avatar lidel commented on June 2, 2024

the tests themselves are going to be quite simple ("send request, check bytes") and that the complex part (which requires libraries, I/Os, etc) will be fixture generation?

Yes, that is why having static fixtures as CARs will simplify things a lot, and we now proactively ask for them in IPIPs.

Example: _redirects IPIP includes fixtures that we use in existing sharness tests.

Sadly, we have some technical debt in gateway tests for older things.

@laurentsenta in case its useful, we may be in a better place if we pay this debt in two steps:

  1. Update all sharness tests for gateway to use static fixtures in CARs, instead of generating them of the fly, and have them all pass.
  2. Reuse the same CARs where, in this new test suite.

It will make migration way less stressful, knowing we are testing the same DAGs.

Is it possible to write a data-driven / declarative test suite for the gateway specs?

I believe so. Static fixtures, clear inputs and outputs. Makes writing assertions straightforward, and we should be able to return actionable error every time.

If we have to choose between the ease of maintenance versus a low barrier to entry for new test writers, which should we pick?

I agree with Robin, but also would say: prioritize future readers :) In gateway context, we write tests once, rarely modify them (ossification-by-design), but run and read results many times.

  • when you need to write new tests, being able to clone repo, look at existing tests and "wing it" (without spending hours on tutorials and guides) makes a big difference in contributor experience
  • looking at tests sometimes clarifies some nuance that was not captured by spec prose
  • running it on own CI and fixing errors until it passes is the fastest way be spec-compliant.

We will have people not familiar with JS tasked with running this suite, often outside PL.
Prioritize clear output, especially when something fails the test.

from gateway-conformance.

laurentsenta avatar laurentsenta commented on June 2, 2024

Thanks for the detailed feedback @darobin, @lidel; That _redirects IPIP got me quite excited, it looks like a good candidate to write conformance tests from the spec (which might be quite different from porting kubo sharness tests).

Update all sharness tests for gateway to use static fixtures in CARs, instead of generating them of the fly, and have them all pass.

That makes sense, I'll look into this!

A few notes, to summarize:

The test suite:

  • must NOT be fragile to dependencies upgrade and breakage.
    • We expect that code to run for years without needing major refactors, rewrites, etc.
  • must output practical error messages.
  • must support car fixtures.
  • must be approachable by "any" dev, even if they are not familiar with stack X" (X being js, go, or anything else).
    • it must be easy to define new tests, run the test suite, and debug errors
  • should let us define tests in a completely declarative way.

It seems like we have 3 blocks:

  • create and prepare fixtures
  • provision the gateway with initial fixtures
  • run the test from a test definition + fixtures

Screenshot 2023-02-15 at 10 53 34

We care the most about the test definitions. We expect thousands of tests, and we don't want to change these often. Other parts, like the provision script, are probably something we can rewrite in a couple of hours.

This is what I have tried recently:

I've been playing around with generating car files as test fixtures, then generating configurations from these fixtures.

A test looks like this:

"GET unixfs file as CAR (by using a single file we ensure deterministic result that can be compared byte-for-byte)":
{
tests: {
"GET with format=car param returns a CARv1 stream": {
url: `/ipfs/${fixture.car._cid}/subdir/ascii.txt?format=car`,
expect: [200, dagAsString(fixture.car.subdir["ascii.txt"])],
},
"GET for application/vnd.ipld.car returns a CARv1 stream": {
url: `/ipfs/${fixture.car._cid}/subdir/ascii.txt`,
headers: { accept: IPLD_CAR_TYPE },
expect: [200, dagAsString(fixture.car.subdir["ascii.txt"])],
},
"GET for application/vnd.ipld.raw version=1 returns a CARv1 stream":
{
url: `/ipfs/${fixture.car._cid}/subdir/ascii.txt`,
headers: { accept: `${IPLD_CAR_TYPE};version=1` },
expect: [200, dagAsString(fixture.car.subdir["ascii.txt"])],
},
"GET for application/vnd.ipld.raw version=1 returns a CARv1 stream (with whitespace)":
{
url: `/ipfs/${fixture.car._cid}/subdir/ascii.txt`,
headers: { accept: `${IPLD_CAR_TYPE}; version=1` },
expect: [200, dagAsString(fixture.car.subdir["ascii.txt"])],
},
"GET for application/vnd.ipld.raw version=2 returns HTTP 400 Bad Request error":
{
url: `/ipfs/${fixture.car._cid}/subdir/ascii.txt`,
headers: { accept: `${IPLD_CAR_TYPE};version=2` },
expect: [400, /unsupported CAR version/],
},
},
},

I took a few screenshots of errors while fixing a test:

Screenshot 2023-02-14 at 16 46 22

Screenshot 2023-02-15 at 10 59 43

Screenshot 2023-02-14 at 16 56 45

So far, it seems to me that:

  • It's a good enough starting point to start adding more tests and learning about the problem space.
  • It's not good enough for the final test suite: some information is hidden in comments like these checks. Ideally, every assert should come with details to show the user. We'll have to augment the library we are using.
  • Using Typescript here has the benefit of giving us "nice" programming features that we wouldn't get from a yaml or json configuration (variable, code reuse, string interpolations, etc).
  • We don't have to buy into the js ecosystem with all its quirks, broken dependencies, and disappearing leftpad libraries. We can stick to a single dependency on our testing module.
  • The part that will become outdated quickly is the part that prepares fixtures (like generating car files using some library that might change, etc). But no matter the stack, we might benefit from keeping this outside the test definitions themselves.

A few next steps: We started from @darobin and @galargh poc and the declarative-e2e-test library format. It might make sense to chat @lidel and see what the project would look like if we started from the approach you have in mind.

from gateway-conformance.

laurentsenta avatar laurentsenta commented on June 2, 2024

@galargh have you had a chance to publish the demo ?
If @darobin is all right with the new approach, using go, we can close this ticket.

from gateway-conformance.

galargh avatar galargh commented on June 2, 2024

@darobin Here's the video from the DEMO. Sorry for the delay. It's been quite a week 😅

from gateway-conformance.

darobin avatar darobin commented on June 2, 2024

No worries at all, we're all busy. It took me a week to get to watch this :)

This looks really nice, thank you! I love the builder pattern. The one (relatively small) thing I would flag is that you seem to be relying on string matching (equals/contains) but a lot of these values can be wilder (eg. case insensitive, there may be spaces, the value might have to be anchored at the start of the string, etc.). If your HTTP library normalises values, this probably helps a fair bit (worth checking), but it's like that quite a few checks will need to go to regex land. This isn't a big deal and not a problem for the framework, I'm just flagging it as a potential problem that jumped out, probably worth addressing before too many tests are written.

Good stuff!

from gateway-conformance.

laurentsenta avatar laurentsenta commented on June 2, 2024

Thanks for the feedback @darobin I created #21 to take care of this.

Closing this ticket since we've reached an agreement,
thanks for all the comments and contributions!

from gateway-conformance.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.