MASQ-Project / MASQ-Node-issues

This repo contains the issues that are used for planning MASQ Node product work. It has no code in it, only GitHub issue tickets
https://masq.ai/
31 stars 12 forks source link

Consider better management for test utilities. #456

Open bertllll opened 3 years ago

bertllll commented 3 years ago

For example for masq we do it right what regards maintaining code that has been created only for testing. Simply because the UI tests live all in the same crate and so the standard cargo means make it done easily. We aren't that efficient in other crates, though. This plays a big role in the biggest crate node.

That said, when we build up our application it consists a large load of unnecessary code which means something only in testing.

The most straightforward way, of course, would be to mark the appropriate code with #[cfg(test)]. Sadly, there is a stopper. We often use features shared between different crates, not only there where were they declared and it is a thing that Rust does not support conditional compilation with the test argument when used for a group of commonly referenced crates. You cannot make use of this root condition. If you try, these conditionally compiled pieces of code will be still effective in the crate where declared but will become invisible for other crates.

Try to find a way how to turn off the big quantum of test code so that it doesn't need to be included in the production binary files.

bertllll commented 1 year ago

We've gone a long way, and so we know much more now.

I actually see it as a narrow set of options. We've got two. 1) We can try to create an isolated crate which we would fill in with just all test utils. We'd be using it as a library to our other crates. In the end, making this library a part of the dev-dependencies place in a cargo.toml could cause it would be ignored at the release compilation. Or we can disable the reference in some other ways, I'm sure there are some. However, this approach might not be even possible because: a) we can run into problems with circular dependencies... b) it's possible that it'll become unmaintainable because for writing these utils it could happen that pieces of code from the production tree are needed which will become a big problem to make them visible for this external crate...any excessive visibility is not a thing we want. 2) We can go the tough but worth-some path with conditional features. I've implemented it a couple of times already and it works nice, if the codebase maintainers know what it is and how to treat it. The base stone is a feature with a constant, already picked name, with which we need to hold on. It's no_test_share. One catch in this can be found in the perspective of use of it. Most of time you want to claim a negation of it, because most of time we're doing testing. That means that so-supposed shared code should bear this attribute above it #[cfg(not(feature = "no_test_share"))]. By this usage we bypass the impotency of #[cfg(test)] at code shared with places outside the original crate. But all good must have also the bad side. Using features in a way like this causes that the IDE will deactivate this piece of code which isn't much fun, increasingly with the growing amount of code marked with it. The trend would be that we'd make our work harder and harder. I know a workaround. It's not the purest one, but which actually is, by definition, right? So I use a combination the classic cargo mark and our own feature. The bright idea in this doing is that both mean "something what is expected to be disabled later in favor of leaner code", and the main trade off is that the IDE reacts well on the [cfg(test)] attribute so it leaves the code activated for as much time as you need. However to make it work smoothly you need to write it like this: #[cfg(any(test, not(feature = "no_test_share")))].

I admit, this complex attribute isn't pretty. But it works.

I know about how we could improve it. By writing a very simple procedural macro (they allows to define custom attributes) which will look simple and have a good name. That macro wouldn't do anything else than it would unwind into the desired combination of attributes that can be seen here above. In my opinion this approach is quite balanced and even it's not going to be extremely comfortable to think about the backyard mixed relationships between different cartes and pieces of code, we might not have any other chance if we want to cut back on some unnecessary, yet bulky code.