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

chore: Add new custom built and test target to make in order to enable easy build or test single nim modules #2913

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

NagyZoltanPeter
Copy link
Contributor

@NagyZoltanPeter NagyZoltanPeter commented Jul 16, 2024

Description

While chating with @fryorcraken, he popped up, why don't we have an easy way of running single tests.
We all do this in different ways, like me I'm using some alias to achieve this.

So I made this little experiment to add custom targets to our makefile to ease and make common and available this task.

While it's a bit tricky to achieve such with make and nimscript I found a usable solution.

So the target is to check if a single file you're working on builds or you may write unit test and want to run first separately from other tests.

With this enhancement, you just need to run

make build/<path-to-your-file> or make test/<path-to-your-file>

Why is it good?

  • no prerequisite knowledge needed how we build our software. All knowledge is coded into Makefile and waku.nimble files.
  • It will always follow the changes made to build arguments anytime in the future.
  • Simplify ramp-up for anybody newly contributing to the project.

Changes

  • Makefile - added build/... and test/... targets
  • Extended waku.nimble with new task that can build any arbitrary single file or even it can run it.

How to test

run:
1a. make build/tests/common/test_tokenbucket.nim
1b. make test/tests/common/test_tokenbucket.nim

Check: ./build/tests/common/test_tokenbucket.nim.bin exists

Suggestions/ideas are welcome

It is not trivial to add parametric target to make, but still possible I'm not aware of the proper solution. If you have any better idea or addon to this, I'm happy to discuss.

Copy link

github-actions bot commented Jul 16, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2913

Built from 8780f63

Copy link
Contributor

@gabrielmer gabrielmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing initiative! Thanks so much! 😍

@gabrielmer
Copy link
Contributor

Now that I think of it, we should mention this on the README so this easier method doesn't get lost/ignored

Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for it ! 💯

Copy link
Contributor

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@NagyZoltanPeter NagyZoltanPeter merged commit ad25f43 into master Jul 17, 2024
10 of 12 checks passed
@NagyZoltanPeter NagyZoltanPeter deleted the chore-add-single-module-build branch July 17, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants