Opened 12 months ago
Last modified 12 months ago
#28743 new defect
Tinytest library leaks out into non-test codebase
Reported by: | karalabe | Owned by: | |
---|---|---|---|
Priority: | Medium | Milestone: | Tor: unspecified |
Component: | Core Tor/Tor | Version: | |
Severity: | Trivial | Keywords: | |
Cc: | nickm | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
This isn't a bug per se, rather a peculiarity in the code organization that I've encountered, which I think is accidental. Please correct me if I'm wrong.
I've been trying to wrap the 0.3.5 series of Tor in Go (https://github.com/ipsn/go-libtor). It's working properly as far as I can tell, just making my final tweaks before pushing it. To keep my wrapper small, I've discarded some of the sources that are not relevant for building the library, namely src/tools
and src/test
.
To my surprise, my build failed (it worked correctly for 0.3.3), and it turned out that the tinytest
library (at least half of it) is in src/ext
, which refers to the src/test
folder that I just deleted (the test folder refers to tinytest, circular dependency, but that's beyond the point here).
Working around it wasn't too big of a deal, I just ignored the tinytest files, but was wondering if this was an accidental leakage of test files into the library part, adding a dependency to the tests? Shouldn't the 4 tinytest files from src/ext
be moved into src/test
?
Child Tickets
Change History (4)
comment:1 Changed 12 months ago by
Cc: | nickm added |
---|---|
Type: | enhancement → defect |
comment:2 Changed 12 months ago by
Hang on though, the mocking macros aren't part of the tinytest code at all. They're in lib/testsupport.
comment:3 Changed 12 months ago by
Tinytest is part of src/ext
per https://gitweb.torproject.org/tor.git/tree/src/ext?h=release-0.3.5 , which seems odd given that it's a test utility (why not put it into src/test
).
The tinytest.c
file contains defines TINYTEST_POSTFORK
in https://gitweb.torproject.org/tor.git/tree/src/ext/tinytest.c?h=release-0.3.5#n28, which causes it to add a link dependency for void tinytest_prefork()
and void tinytest_postfork()
per https://gitweb.torproject.org/tor.git/tree/src/ext/tinytest.c?h=release-0.3.5#n122 .
Those methods are defined in src/test/testing_common.c
per https://gitweb.torproject.org/tor.git/tree/src/test/testing_common.c?h=release-0.3.5#n230 .
My annoyance is really with code organization, that a file in a library requires stuff defined in a test harness.
comment:4 Changed 12 months ago by
Milestone: | → Tor: unspecified |
---|
Thanks for using Tor, and for reporting this issue.
Tor depends on tinytest's mocking macros.
The macros don't do anything in non-test code, but they're still a compile-time dependency.
But it's weird that tinytest refers to src/test. We don't want circular dependencies like that.
nickm wrote tinytest, I'll see if he has any more insight.
Tor's in-process API is quite new, and we're still testing it.
If you find any other bugs, please let us know.