Nim ping test

[Menduist]

Should work fine now! No metrics though

Old message:

---

Works fine against itself, could use a hand to setup the interop tests themselves

I'm also not sure what format the pubsub payload should have, because it's being escaped
If I send {"publish": {"topic": "whatev", "payload": {"test": "whatev"}}
I receive something like "{"test": "whatev"}"

So I sent it as a string, which may fail with other implementations of the SDK

Also missing metrics since I don't support them in the SDK (yet)

[laurentsenta]

Awesome, thanks for sharing,

If you want to give this a shot:
you could create a all-interop-latest.toml, copy the content of https://github.com/libp2p/test-plans/blob/master/ping/_compositions/go-rust-interop-latest.toml and add a [groups] section with the nim builder. The section should look something like:

test-plans/ping/_compositions/go-rust-interop-latest.toml

Lines 18 to 29 in b97b36a

	[[groups]]
	id = "go-latest-{{ .Id }}"
	instances = { count = 1 }
	builder = "docker:go"
	[groups.build]
	selectors = ['{{ .Selector }}']
	[groups.build_config]
	path = "./go/"
	build_base_image = 'golang:{{ .GoVersion }}-buster'
	modfile = "{{ .Modfile }}"

(ignore templating for now).

On the payload:
that's how the sync service works at the moment (on my todo is to describe the current, implicit, spec).
You get the JSON back, encoded as a string.

Example in rust:
https://github.com/testground/sdk-rust/blob/2b85e4c9578037bd8704de40b7f179bf7fcc8649/src/responses.rs#L110

[Menduist]

It works!

Had to fix a few things (the string thing, and the nim SDK doesn't signal initialized_global which is apparently required for the other SDKs, so I did it manually here), but seems to work fine now :)

Will have to find how to handle nim-libp2p versions properly, only need to specify a tag or a specific commit in the docker file here:

test-plans/ping/nim/Dockerfile

Lines 3 to 4 in c8e4b32

	RUN nimble install -y "https://github.com/status-im/testground-nim-sdk@#c282ff68c08ef85a7ca011e077e3e69eb1a6edec"
	RUN nimble install -y "libp2p"

This is how it's used in the testground nim sdk CI:
https://github.com/status-im/testground-nim-sdk/blob/c282ff68c08ef85a7ca011e077e3e69eb1a6edec/examples/simple_tcp_ping/Dockerfile#L6

But not sure how it integrates into all the templating stuff

[laurentsenta]

Nice!

If all you need is a git commit, recommendation:
create a nim.toml which specifies multiple groups with an Id and a GitRef value.

Check the rust part, I guess it would look something like:

{{ with (load_resource "./nim.toml") }}
  {{ with (index .groups 0) }} // get the latest
    [[groups]]
    id = "nim-latest-{{ .Id }}"
    instances = { count = 1 }
    builder = "docker:generic"

    [groups.build_config]
      path = "./rust/"

    [groups.build_config.build_args]
      GIT_REF = '{{ .GitRef }}'
  {{ end }}
{{ end }}

[Menduist]

Done, + an attempt at adding to CI

[Menduist]

Workflow ran successfully on the fork: https://github.com/status-im/libp2p-test-plans/actions/runs/3427378367
So I would say this is ready for review

[mxinden]

🚀 happy to see this happening.

[mxinden]

Instead of introducing a new workflow file, how about including nim in ping-interop.yml. I don't see a reason why we should treat Go or Rust special.

Maybe @MarcoPolo has opinions here.

[Menduist]

ping-interop.yml will call the go-rust-interop.toml composition
So it could just call the all-interop.toml instead, but then the go-rust-interop.toml will never be called, and maybe it should be removed?
We would just have:

• [language].toml: list of versions of that implem
• [language]-cross-versions.toml: test every version of that implem
• all-interop-latest.toml: test last version of every implem
• all-interop.toml: test every version of every implem

[Menduist]

Did this in my latest commit, we can revert if not happy

[mxinden]

We would just have:

* `[language].toml`: list of versions of that implem

* `[language]-cross-versions.toml`: test every version of that implem

* `all-interop-latest.toml`: test last version of every implem

* `all-interop.toml`: test every version of every implem

On a high level I am in favor of this. I will take a detailed look now.

[mxinden]

Very excited for us to have interoperability tests beyond Go, JS and Rust.

The proposed changes here look good to me.

I suggest waiting for another review from @MarcoPolo (as self-requested).

In addition, I wonder how we should coordinate this with #53, i.e. how we should prioritize this and the refactoring of our test matrix generation. As far as I can tell, it would be fine to proceed here before #53. What do you think @jxs and @MarcoPolo?

[Menduist]

Soo the CI started to fail because of disk space usage:
https://github.com/status-im/libp2p-test-plans/actions/runs/3535301751
AFAICT, the Nim images are less than 1gb, and runners should have at least 14gb of disk available: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
So I'm just unlucky enough to be the guy to reach this :D There are apparently some tricks to have more space in the runner, but using 14gb to build the images seems a lot to me, are the other implementation caching their dependencies properly to be reused by multiple images?

[mxinden]

are the other implementation caching their dependencies properly to be reused by multiple images?

Would you mind merging master here @Menduist? #78 might resolve the issues you are seeing.

[mxinden]

I suggest waiting for another review from @MarcoPolo (as self-requested).

Friendly ping @MarcoPolo. Any objections moving forward here?

[mxinden]

Triggered the tests. All green @Menduist 🎉

[MarcoPolo]

Looks great! @Menduist I'll ping you for some help in getting the multidimensional testing in the near future. Maybe we can pair on it? Might be fun to pair on some Nim 👑 👑 👑

[Menduist]

Sure! Thanks for the merge 🎉