You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by Adam Kocoloski <ko...@apache.org> on 2021/11/23 20:48:21 UTC

[DISCUSS] EUnit test files location

Hi,

TL;DR - I think it would be a good idea to move EUnit tests back into `test` instead of `test/eunit`.

Recently I’ve been spending some time porting specific libraries to use rebar3 and I thought it might be a nice project to hack on supporting rebar3 in CouchDB proper during the holiday weekend. I noticed a few small issues, but one in particular is that it was difficult to get rebar3 to pick up unit tests that were in subdirectories of `test`.

Eventually I figured out that you could tell the compiler to do recursive compilation on these test directories and that worked for running the end-to-end test suite, but then I found that rebar3’s `—app` flag was still broken, e.g. if I ran `rebar3 eunit —app couch_log` it would only find tests in the actual source modules but not any of the modules in the test/eunit directory. I thought this might be an inconsistency bug in rebar3, but I realized that when you invoked rebar3 this way it was really just calling `eunit:test({application, couch_log})`, and EUnit doesn’t allow any configurability of its search path when invoked this way.

Bottom line, it feels like we’re going against the grain of eunit itself when we try to move the eunit tests to anywhere else other than `test`. Currently we have a small handful of apps following this pattern:

chttpd
couch
couch_epi
couch_log
couch_prometheus
couch_replicator

Of those, only the first two have any ExUnit unit tests that (I think) motivated the creation of separate eunit and exunit tests in the first place. Would it be OK if we moved those test modules back into `test` so we can stay on the “paved path” for EUnit usage going forward?

Cheers, Adam

Re: [DISCUSS] EUnit test files location

Posted by Nick Vatamaniuc <va...@gmail.com>.
Using rebar 3.15.2 would work for now. Good find.

For _build directory, a good number of dependencies already have their
own unit tests for PRs, so they should be ok. Some we just skip
altogether anyway
https://github.com/apache/couchdb/blob/main/Makefile#L76
(folsom,meck,mochiweb,triq,proper,snappy,bcrypt,hyper,ibrowse).

But the issue is that those dependencies just won't run their own
tests or that they won't even be available as dependencies for other
eunit tests. If it's later, we may have to do some additional magic,
like you suggested.

Thanks,
-Nick

On Fri, Nov 26, 2021 at 8:57 AM Adam Kocoloski <ko...@apache.org> wrote:
>
> The pc plugin has handled everything I’ve thrown at it so far. I recall Paul mentioning some issue with transitive dependencies; I’ll keep an eye out for any trouble. I didn’t try anything with Fauxton and docs yet.
>
> Rebar3 supported Erlang 20 as recently as 3.15.2, so I can imagine an option to just use that version for Erlang 20 / 21 builds and the latest version for 22+. That’s what I ended up doing for the GH Actions CI for the b64url lib. For what it’s worth, the rebar3 team only intends to support 3 major releases at a time going forward, see https://github.com/erlang/rebar3/pull/2555 <https://github.com/erlang/rebar3/pull/2555>. They argue that this is consistent with the support statement from Erlang/OTP itself.
>
> One notable difference I’ve found so far is that our current setup considers dependencies to be peers to the internal apps in the umbrella couchdb repo: we put all of them in src/, we run their tests as part of our test suite, etc. Rebar3 installs dependencies to the _build directory and does not test them. If it’s essential that we consider code in these other repositories to be part of an Apache CouchDB source release we might need to get a little creative.
>
> As an aside, I didn’t intend to weigh in on the rebar3 vs. mix debate here. I can see good arguments for both. I just found myself gaining some familiarity with rebar3 recently, and figured there would be a fair amount of common work involved in aligning our build system to either modern option.
>
> Cheers, Adam
>
> > On Nov 24, 2021, at 2:07 PM, Nick Vatamaniuc <va...@gmail.com> wrote:
> >
> > Good idea to try rebar3! I tried it a few years back and got stumped
> > at building some NIFs and a few other issues. Most of those can be
> > overcome with a bit of work.
> >
> > I saw you used the pc compiler for NIFs, and I think rebar3 can set
> > overrides now too
> > https://github.com/blt/port_compiler#use-with-existing-dependency for
> > individual dependencies. Perhaps the issue I encountered might be
> > solved that way.
> >
> > The other issue was our "raw" dependencies - fauxton and docs. The
> > approach I had tried was to make them look like Erlang apps. I don't
> > recall how successful that was. But I think there are some newer
> > things like https://rebar3.readme.io/docs/custom-dep-resources which
> > we could leverage there.
> >
> > One more issue I recently discovered was that rebar3 is not Erlang 20
> > compatible. We ship our releases with the Erlang 20 as the minimum
> > version and if we update to use rebar3 our release branch might be
> > left behind. If the issue is just using Tag:Error:Stack catch
> > patterns, we could potentially have our own fork of rebar3, as we
> > already do for rebar2.
> >
> > Cheers,
> > -Nick
> >
> >
> > On Tue, Nov 23, 2021 at 5:39 PM Adam Kocoloski <ko...@apache.org> wrote:
> >>
> >> Bah, ignore the message below. The issue isn’t that EUnit has a problem with nested directories, it’s that it expects each test module to have a `_tests` suffix, not a `_test` one. We’re inconsistent about adhering to that rule in our codebase I’ll submit a PR to fix the module names so no one else bumps into that issue.
> >>
> >> Adam
> >>
> >>> On Nov 23, 2021, at 3:48 PM, Adam Kocoloski <ko...@apache.org> wrote:
> >>>
> >>> Hi,
> >>>
> >>> TL;DR - I think it would be a good idea to move EUnit tests back into `test` instead of `test/eunit`.
> >>>
> >>> Recently I’ve been spending some time porting specific libraries to use rebar3 and I thought it might be a nice project to hack on supporting rebar3 in CouchDB proper during the holiday weekend. I noticed a few small issues, but one in particular is that it was difficult to get rebar3 to pick up unit tests that were in subdirectories of `test`.
> >>>
> >>> Eventually I figured out that you could tell the compiler to do recursive compilation on these test directories and that worked for running the end-to-end test suite, but then I found that rebar3’s `—app` flag was still broken, e.g. if I ran `rebar3 eunit —app couch_log` it would only find tests in the actual source modules but not any of the modules in the test/eunit directory. I thought this might be an inconsistency bug in rebar3, but I realized that when you invoked rebar3 this way it was really just calling `eunit:test({application, couch_log})`, and EUnit doesn’t allow any configurability of its search path when invoked this way.
> >>>
> >>> Bottom line, it feels like we’re going against the grain of eunit itself when we try to move the eunit tests to anywhere else other than `test`. Currently we have a small handful of apps following this pattern:
> >>>
> >>> chttpd
> >>> couch
> >>> couch_epi
> >>> couch_log
> >>> couch_prometheus
> >>> couch_replicator
> >>>
> >>> Of those, only the first two have any ExUnit unit tests that (I think) motivated the creation of separate eunit and exunit tests in the first place. Would it be OK if we moved those test modules back into `test` so we can stay on the “paved path” for EUnit usage going forward?
> >>>
> >>> Cheers, Adam
> >>
>

Re: [DISCUSS] EUnit test files location

Posted by Adam Kocoloski <ko...@apache.org>.
The pc plugin has handled everything I’ve thrown at it so far. I recall Paul mentioning some issue with transitive dependencies; I’ll keep an eye out for any trouble. I didn’t try anything with Fauxton and docs yet.

Rebar3 supported Erlang 20 as recently as 3.15.2, so I can imagine an option to just use that version for Erlang 20 / 21 builds and the latest version for 22+. That’s what I ended up doing for the GH Actions CI for the b64url lib. For what it’s worth, the rebar3 team only intends to support 3 major releases at a time going forward, see https://github.com/erlang/rebar3/pull/2555 <https://github.com/erlang/rebar3/pull/2555>. They argue that this is consistent with the support statement from Erlang/OTP itself.

One notable difference I’ve found so far is that our current setup considers dependencies to be peers to the internal apps in the umbrella couchdb repo: we put all of them in src/, we run their tests as part of our test suite, etc. Rebar3 installs dependencies to the _build directory and does not test them. If it’s essential that we consider code in these other repositories to be part of an Apache CouchDB source release we might need to get a little creative.

As an aside, I didn’t intend to weigh in on the rebar3 vs. mix debate here. I can see good arguments for both. I just found myself gaining some familiarity with rebar3 recently, and figured there would be a fair amount of common work involved in aligning our build system to either modern option.

Cheers, Adam

> On Nov 24, 2021, at 2:07 PM, Nick Vatamaniuc <va...@gmail.com> wrote:
> 
> Good idea to try rebar3! I tried it a few years back and got stumped
> at building some NIFs and a few other issues. Most of those can be
> overcome with a bit of work.
> 
> I saw you used the pc compiler for NIFs, and I think rebar3 can set
> overrides now too
> https://github.com/blt/port_compiler#use-with-existing-dependency for
> individual dependencies. Perhaps the issue I encountered might be
> solved that way.
> 
> The other issue was our "raw" dependencies - fauxton and docs. The
> approach I had tried was to make them look like Erlang apps. I don't
> recall how successful that was. But I think there are some newer
> things like https://rebar3.readme.io/docs/custom-dep-resources which
> we could leverage there.
> 
> One more issue I recently discovered was that rebar3 is not Erlang 20
> compatible. We ship our releases with the Erlang 20 as the minimum
> version and if we update to use rebar3 our release branch might be
> left behind. If the issue is just using Tag:Error:Stack catch
> patterns, we could potentially have our own fork of rebar3, as we
> already do for rebar2.
> 
> Cheers,
> -Nick
> 
> 
> On Tue, Nov 23, 2021 at 5:39 PM Adam Kocoloski <ko...@apache.org> wrote:
>> 
>> Bah, ignore the message below. The issue isn’t that EUnit has a problem with nested directories, it’s that it expects each test module to have a `_tests` suffix, not a `_test` one. We’re inconsistent about adhering to that rule in our codebase I’ll submit a PR to fix the module names so no one else bumps into that issue.
>> 
>> Adam
>> 
>>> On Nov 23, 2021, at 3:48 PM, Adam Kocoloski <ko...@apache.org> wrote:
>>> 
>>> Hi,
>>> 
>>> TL;DR - I think it would be a good idea to move EUnit tests back into `test` instead of `test/eunit`.
>>> 
>>> Recently I’ve been spending some time porting specific libraries to use rebar3 and I thought it might be a nice project to hack on supporting rebar3 in CouchDB proper during the holiday weekend. I noticed a few small issues, but one in particular is that it was difficult to get rebar3 to pick up unit tests that were in subdirectories of `test`.
>>> 
>>> Eventually I figured out that you could tell the compiler to do recursive compilation on these test directories and that worked for running the end-to-end test suite, but then I found that rebar3’s `—app` flag was still broken, e.g. if I ran `rebar3 eunit —app couch_log` it would only find tests in the actual source modules but not any of the modules in the test/eunit directory. I thought this might be an inconsistency bug in rebar3, but I realized that when you invoked rebar3 this way it was really just calling `eunit:test({application, couch_log})`, and EUnit doesn’t allow any configurability of its search path when invoked this way.
>>> 
>>> Bottom line, it feels like we’re going against the grain of eunit itself when we try to move the eunit tests to anywhere else other than `test`. Currently we have a small handful of apps following this pattern:
>>> 
>>> chttpd
>>> couch
>>> couch_epi
>>> couch_log
>>> couch_prometheus
>>> couch_replicator
>>> 
>>> Of those, only the first two have any ExUnit unit tests that (I think) motivated the creation of separate eunit and exunit tests in the first place. Would it be OK if we moved those test modules back into `test` so we can stay on the “paved path” for EUnit usage going forward?
>>> 
>>> Cheers, Adam
>> 


Re: [DISCUSS] EUnit test files location

Posted by Nick Vatamaniuc <va...@gmail.com>.
Good idea to try rebar3! I tried it a few years back and got stumped
at building some NIFs and a few other issues. Most of those can be
overcome with a bit of work.

I saw you used the pc compiler for NIFs, and I think rebar3 can set
overrides now too
https://github.com/blt/port_compiler#use-with-existing-dependency for
individual dependencies. Perhaps the issue I encountered might be
solved that way.

The other issue was our "raw" dependencies - fauxton and docs. The
approach I had tried was to make them look like Erlang apps. I don't
recall how successful that was. But I think there are some newer
things like https://rebar3.readme.io/docs/custom-dep-resources which
we could leverage there.

One more issue I recently discovered was that rebar3 is not Erlang 20
compatible. We ship our releases with the Erlang 20 as the minimum
version and if we update to use rebar3 our release branch might be
left behind. If the issue is just using Tag:Error:Stack catch
patterns, we could potentially have our own fork of rebar3, as we
already do for rebar2.

Cheers,
-Nick


On Tue, Nov 23, 2021 at 5:39 PM Adam Kocoloski <ko...@apache.org> wrote:
>
> Bah, ignore the message below. The issue isn’t that EUnit has a problem with nested directories, it’s that it expects each test module to have a `_tests` suffix, not a `_test` one. We’re inconsistent about adhering to that rule in our codebase I’ll submit a PR to fix the module names so no one else bumps into that issue.
>
> Adam
>
> > On Nov 23, 2021, at 3:48 PM, Adam Kocoloski <ko...@apache.org> wrote:
> >
> > Hi,
> >
> > TL;DR - I think it would be a good idea to move EUnit tests back into `test` instead of `test/eunit`.
> >
> > Recently I’ve been spending some time porting specific libraries to use rebar3 and I thought it might be a nice project to hack on supporting rebar3 in CouchDB proper during the holiday weekend. I noticed a few small issues, but one in particular is that it was difficult to get rebar3 to pick up unit tests that were in subdirectories of `test`.
> >
> > Eventually I figured out that you could tell the compiler to do recursive compilation on these test directories and that worked for running the end-to-end test suite, but then I found that rebar3’s `—app` flag was still broken, e.g. if I ran `rebar3 eunit —app couch_log` it would only find tests in the actual source modules but not any of the modules in the test/eunit directory. I thought this might be an inconsistency bug in rebar3, but I realized that when you invoked rebar3 this way it was really just calling `eunit:test({application, couch_log})`, and EUnit doesn’t allow any configurability of its search path when invoked this way.
> >
> > Bottom line, it feels like we’re going against the grain of eunit itself when we try to move the eunit tests to anywhere else other than `test`. Currently we have a small handful of apps following this pattern:
> >
> > chttpd
> > couch
> > couch_epi
> > couch_log
> > couch_prometheus
> > couch_replicator
> >
> > Of those, only the first two have any ExUnit unit tests that (I think) motivated the creation of separate eunit and exunit tests in the first place. Would it be OK if we moved those test modules back into `test` so we can stay on the “paved path” for EUnit usage going forward?
> >
> > Cheers, Adam
>

Re: [DISCUSS] EUnit test files location

Posted by Adam Kocoloski <ko...@apache.org>.
Bah, ignore the message below. The issue isn’t that EUnit has a problem with nested directories, it’s that it expects each test module to have a `_tests` suffix, not a `_test` one. We’re inconsistent about adhering to that rule in our codebase I’ll submit a PR to fix the module names so no one else bumps into that issue.

Adam

> On Nov 23, 2021, at 3:48 PM, Adam Kocoloski <ko...@apache.org> wrote:
> 
> Hi,
> 
> TL;DR - I think it would be a good idea to move EUnit tests back into `test` instead of `test/eunit`.
> 
> Recently I’ve been spending some time porting specific libraries to use rebar3 and I thought it might be a nice project to hack on supporting rebar3 in CouchDB proper during the holiday weekend. I noticed a few small issues, but one in particular is that it was difficult to get rebar3 to pick up unit tests that were in subdirectories of `test`.
> 
> Eventually I figured out that you could tell the compiler to do recursive compilation on these test directories and that worked for running the end-to-end test suite, but then I found that rebar3’s `—app` flag was still broken, e.g. if I ran `rebar3 eunit —app couch_log` it would only find tests in the actual source modules but not any of the modules in the test/eunit directory. I thought this might be an inconsistency bug in rebar3, but I realized that when you invoked rebar3 this way it was really just calling `eunit:test({application, couch_log})`, and EUnit doesn’t allow any configurability of its search path when invoked this way.
> 
> Bottom line, it feels like we’re going against the grain of eunit itself when we try to move the eunit tests to anywhere else other than `test`. Currently we have a small handful of apps following this pattern:
> 
> chttpd
> couch
> couch_epi
> couch_log
> couch_prometheus
> couch_replicator
> 
> Of those, only the first two have any ExUnit unit tests that (I think) motivated the creation of separate eunit and exunit tests in the first place. Would it be OK if we moved those test modules back into `test` so we can stay on the “paved path” for EUnit usage going forward?
> 
> Cheers, Adam