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

test: Introduce iroh-test with common logging infrastructure #1365

Merged
merged 9 commits into from
Aug 24, 2023

Conversation

flub
Copy link
Contributor

@flub flub commented Aug 16, 2023

Description

This code was copy-pasted around too much. This takes a simple and
clean approach to this by creating a new crate which is used as a
dev-dependency of all the other crates.

The upside is there is no messing about with cargo features and
delicate matching versions.

The main downside is that it can not provide testing utilities which
build on the code in the other crates. Most notable this is currently
the testing derp server.

Notes & open questions

This skirts around the tricky bits of adding such a common module: The
testing derp server is still copy-pasted around. I think this is
fine, because solving that is ugly. Let's see if any other code ends
up in this crate or if the majority needs a more complicated solution
anyway.

This also doesn't introduce a general published iroh-common crate
(or whatever it would be named). This might also be useful but
affects the public API and introduces other questions. There's no
harm in considering this to be an entirely independent problem and to
later add such a module as well as this one.

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

This code was copy-pasted around too much.  This takes a simple and
clean approach to this by creating a new crate which is used as a
dev-dependency of all the other crates.

The upside is there is no messing about with cargo features and
delicate matching versions.

The main downside is that it can not provide testing utilities which
build on the code in the other crates.  Most notable this is currently
the testing derp server.
Copy link
Contributor

@ramfox ramfox left a comment

Choose a reason for hiding this comment

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

F-yes, this was so annoying, thank you for taking the initiative to fix it.

iroh-net/Cargo.toml Outdated Show resolved Hide resolved
iroh/Cargo.toml Outdated Show resolved Hide resolved
- add version to dependency lines
- add description to crate
- add license info to readme
@flub flub requested review from dignifiedquire and removed request for Frando, rklaehn and divagant-martian August 24, 2023 07:53
@flub flub enabled auto-merge August 24, 2023 08:46
@flub flub added this pull request to the merge queue Aug 24, 2023
Merged via the queue into main with commit 411e20b Aug 24, 2023
15 checks passed
@dignifiedquire dignifiedquire deleted the flub/iroh-test-crate branch November 1, 2023 14:27
matheus23 pushed a commit that referenced this pull request Nov 14, 2024
## Description

This code was copy-pasted around too much.  This takes a simple and
clean approach to this by creating a new crate which is used as a
dev-dependency of all the other crates.

The upside is there is no messing about with cargo features and
delicate matching versions.

The main downside is that it can not provide testing utilities which
build on the code in the other crates.  Most notable this is currently
the testing derp server.


## Notes & open questions

This skirts around the tricky bits of adding such a common module: The
testing derp server is still copy-pasted around.  I think this is
fine, because solving that is ugly.  Let's see if any other code ends
up in this crate or if the majority needs a more complicated solution
anyway.

This also doesn't introduce a general *published* iroh-common crate
(or whatever it would be named).  This might also be useful but
affects the public API and introduces other questions.  There's no
harm in considering this to be an entirely independent problem and to
later add such a module as well as this one.


## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants