-
Notifications
You must be signed in to change notification settings - Fork 261
chore_: running CI with nwaku #6768
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
base: develop
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (61)
|
LIBWAKU := $(CURDIR)/vendor/github.com/waku-org/waku-go-bindings/third_party/nwaku/build/libwaku.$(LIBWAKU_EXT) | ||
$(LIBWAKU): | ||
ifeq ($(USE_NWAKU),true) | ||
ifeq (true,true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be reverted once we want to merge the PR
once we start using nwaku as a default we will need to remove the if condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ivansete-status : you can start here, this flag is available in CI
status-go/_assets/ci/Jenkinsfile.nwaku
Line 96 in 790ef72
shell('USE_NWAKU=true make statusgo-shared-library') |
var err error | ||
s.messagingEnv, err = messaging.NewTestMessagingEnvironment() | ||
s.Require().NoError(err) | ||
s.Require().NoError(s.messagingEnv.Setup()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing this because it just started the node, and the node gets already started in NewTestMessagingEnvironment
starting the node twice was giving issues. nwaku has to protect agains it but in any case, it's a bug to attempt to do it
@@ -1,5 +1,5 @@ | |||
//go:build !use_nwaku |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in order to run the CI and test, I changed the default to be nwaku instead of go-waku. That's the changes in the tags in most of the files in this PR.
Once the PR is ready to merge, we should rever this changes in keep go-waku as the default. Probably add a CI job running everything for nwaku
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nwaku functionalities are tested in waku-go-bindings
. We should check if there's anything covered in the go-waku
test without the equivalent being covered in waku-go-bindings
's tests and add it to this file. The redundant tests shouldn't be here
7648f03
to
f91e5d2
Compare
Draft PR to test running the CI with nwaku. Once it passes it will be cleaned up