couchdb icon indicating copy to clipboard operation
couchdb copied to clipboard

Consolidate various elixir makefile targets

Open kocolosk opened this issue 3 years ago • 4 comments

Summary

If anyone is interested in consolidating the elixir, elixir-only, and elixir-suite into a single target I'm all ears. The first two are identical save for the MIX_ENV and COUCHDB_TEST_ADMIN_PARTY_OVERRIDE environment settings, so I'm guessing we only need one target. The last one uses a skip file instead of the @pending tag; I'm not sure we need two distinct approaches there.

Desired Behaviour

A simpler, more obvious Makefile

Additional context

Originally reported in #3818

kocolosk avatar Nov 10 '21 02:11 kocolosk

Some things I learned:

  • The skip.elixir file has a high degree of overlap with :pending tag, at least on main. Removing the contents of the file causes 13 additional tests to run successfully without introducing any failures.
  • mix will automatically skip any tests with a :skip tag and mark them as skipped; we are only using this in one location today.
  • We additionally configure our test runner to exclude tests with the :pending tag; we have 135 of those on main today. They are marked as excluded instead of skipped in the test output.
  • The suite.elixir file was slightly out-of-date and missing a couple of tests; it's important to regenerate this file periodically via
MIX_ENV=integration mix suite > test/elixir/test/config/suite.elixir

kocolosk avatar Nov 12 '21 14:11 kocolosk

IRC skip.elixir was invented for a case when integrator want to disable tests without touching the upstream CouchDB source code.

iilyak avatar Nov 15 '21 15:11 iilyak

That makes sense @iilyak , thanks. I filed a PR in #3831 a few days ago to drive some improvements but didn't touch the basic skip.elixir / suite.elixir setup. I've been mulling over some ways that we might improve that and I had a few ideas. Would be curious to get your thoughts:

Option 1: Ignore suite.elixir if skip.elixir is empty

If we did this then anyone not relying on a skip.elixir file would automatically pick up new / changed tests. Folks who are relying on the skip.elixir file would still be required to maintain suite.elixir.

Option 2: Dynamically generate suite.elixir content if skip.elixir is used

With this approach no one needs to maintain suite.elixir. Calling Couch.Test.Suite.list() takes about 15 seconds on my machine; most of it is spent require-ing the .exs files, so one might imagine that the subsequent ExUnit run would be faster as a result. Anecdotally it seems 7-10 seconds faster (i.e. 5-8 seconds slower overall) but I haven't tried too hard to measure it.

Option 3: Exclude tests by name

I discovered that the module and test name are automatically injected as tags on each test. That means we could exclude tests by name using the existing skip.elixir format, but we'd have to take care to ensure globally-unique test names (today we have some identically-named tests across modules). No one need maintain suite.elixir, and there's no measurable performance penalty.

kocolosk avatar Nov 15 '21 17:11 kocolosk

Reopen to continue discussion of additional improvements.

kocolosk avatar Nov 15 '21 18:11 kocolosk

We settled on make elixir by renaming elixir-suite -> elixir. ExUnit never took off and the target doesn't actually run any tests we should clean that up. But otherwise, we're down to a single working target in makefile so closing as completed.

nickva avatar Nov 13 '23 18:11 nickva