bxparks / AUnit

Unit testing framework for Arduino platforms inspired by ArduinoUnit and Google Test. Used with EpoxyDuino for continuous builds.
MIT License
179 stars 16 forks source link

Feature request (and patch) : filter test on command line #76

Closed hsaturn closed 3 years ago

hsaturn commented 3 years ago

Hi Brian,

As I really need to run or debug only one test, I really miss the command line filtering option as it is possible with gtest.

So I've written this patch:

I'd be cool to add this patch into the next release.

Best regards

diff --git a/src/aunit/Test.cpp b/src/aunit/Test.cpp
index c06f860..dae8f8e 100644
--- a/src/aunit/Test.cpp
+++ b/src/aunit/Test.cpp
@@ -70,6 +70,26 @@ void Test::insert() {
   *p = this;
 }

+uint8_t Test::getLifeCycle() const
+{
+  if (mLifeCycle != kLifeCycleFinished and not allowed())
+    return kLifeCycleExcluded;
+
+  return mLifeCycle;
+}
+
+bool Test::allowed() const
+{
+  if (epoxy_argc==1) return true;
+  const char* testName = getName().getCString();
+  for(int i=1; i<epoxy_argc; i++)
+  {
+    if (strstr(testName, epoxy_argv[i]) != NULL)
+      return true;
+  }
+  return false;
+}
+
 void Test::resolve() {
   const __FlashStringHelper* const TEST_STRING = F("Test ");

diff --git a/src/aunit/Test.h b/src/aunit/Test.h
index 9dbd54f..0fe1785 100644
--- a/src/aunit/Test.h
+++ b/src/aunit/Test.h
@@ -158,7 +158,7 @@ class Test {
     const internal::FCString& getName() const { return mName; }

     /** Get the life cycle state of the test. */
-    uint8_t getLifeCycle() const { return mLifeCycle; }
+    uint8_t getLifeCycle() const;

     void setLifeCycle(uint8_t state) { mLifeCycle = state; }

@@ -277,6 +277,9 @@ class Test {
     /** Get the verbosity. */
     uint8_t getVerbosity() const { return mVerbosity; }

+    /** Check if test is not filtered by the command line **/
+    bool allowed() const;
+
   private:
     // Disable copy-constructor and assignment operator
     Test(const Test&) = delete;
hsaturn commented 3 years ago

With that patch it is possible to filter test based on part of their name

aunit> run-tests
Test list_add passed.
Test list_clear passed
Test vector_add passed.
Test vector_clear passed.
Test map_add passed.
Test map_clear passed.

auint> run-tests list
Test list_add passed.
Test list_clear passed
Test vector_add skipped. <-
Test vector_clear skipped. <-
Test map_add skipped. <-
Test map_clear skipped. <-

aunit> run-tests add
Test list_add passed.
Test list_clear skipped. <-
Test vector_add passed.
Test vector_clear skipped. <-
Test map_add passed.
Test map_clear skipped. <-

And last: authorize many test

aunit> run-tests list map
Test list_add passed.
Test list_clear passed.
Test vector_add skipped. <-
Test vector_clear skipped. <-
Test map_add passed.
Test map_clear passed.
bxparks commented 3 years ago

This is a good idea. You are using the new epoxy_argc and epoxy_argv variables that I added only a week ago, which of course works only under EpoxyDuino.

Google Test uses a --gtest_filter flag which supports globbing wildcards and negation (https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests). That's probably far more more than what we need. So I'm thinking of something that supports a list of includes and excludes, like this:

$ ./test.out [include1] [include2] [...] [^exclude1] [^exclude2] [...]

I propose using ^ to mean "exclude", because a minus sign - normally means a command line flag. There is some precedence for that meaning since ^ is used for negation in the [^abc] regular expression. There are only a few other characters without special meaning to the bash shell (@, %, /, =, +) but they don't seem as compelling as ^.

I can't work on this immediately, but maybe in the next few days.

hsaturn commented 3 years ago

Hi Brian

Yes, gtest is more flexible, but I do agree that this is far more than we need.

I though also about adding an exclusion, not sure I'd use it, but this is not a big deal.

I'm not sure ^excluded is a good idea, just because one could think that this is a regex (start by) In fact, I even thought using regexes before returning back to a simple strstr() for sake of simplicity (just what I need). maybe "-X file" (like tar exclude option) (with -x also) ?

I have committed my patch on my local clone. I have this feature for my usage and from now will git pull --rebase to keep it. So you may (or never) implement this the way you like:-)

Best regards, and thousands of thanks for AUnit which is just great tool to ensure non regression for my libraries.

Francois Biot

Le lun. 20 sept. 2021 à 20:00, Brian Park @.***> a écrit :

This is a good idea. You are using the new epoxy_argc and epoxy_argv variables that I added only a week ago, which of course works only under EpoxyDuino.

Google Test uses a --gtest_filter flag which supports globbing wildcards and negation ( https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests). That's probably far more more than what we need. So I'm thinking of something that supports a list of includes and excludes, like this:

$ ./test.out [include1] [include2] [...] [^exclude1] [^exclude2] [...]

I propose using ^ to mean "exclude", because a minus sign - normally means a command line flag. There is some precedence for that meaning since ^ is used for negation in the [^abc] regular expression. There are only a few other characters without special meaning to the bash shell (@, %, /, =, +) but they don't seem as compelling as ^.

I can't work on this immediately, but maybe in the next few days.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bxparks/AUnit/issues/76#issuecomment-923152024, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADRCSG6DPRIQACY2KRJMXXTUC5ZE5ANCNFSM5EKJIWXA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

bxparks commented 3 years ago

I have needed an exclusion list when I am refactoring an existing code base, and the new code temporarily breaks only a few unit tests, but passes all the others. In this situation, it was useful to run the other tests, while excluding just a few broken tests, until the new code fixes the broken tests.

But I have also been in situations like yours where I wanted to run just a few tests using an include list.

It occurred to me that an include list would never be used at the same time as an exclude list. Because an include list implies that all tests are initially excluded by default. But an exclude list implies that all tests are included by default. So your -x (and its longer form --exclude) would force the argument list to be interpreted as an exclude list. The command line syntax would be:

$ ./test.out [-x | --exclude] [list ...]

This avoids using the special ^ prefix, which allows normal regular expressions to be supported in the future if needed.

bxparks commented 3 years ago

On second thought, I forgot that your implementation uses strstr() which is effectively doing a *pattern* match on the test name. So it is possible for someone to want both an include list and an exclude list at the same time.

The other slight complication is that there is already the TestRunner::include() and TestRunner::exclude() functions which perform an exact match using strcmp(), or a prefix match using strncmp() if there is a trailing *. It would be nice to have some consistency among these options.

How about this: We define 4 flags, each taking a comma-separated list of names/words. We also allow space-separated list of words as you have in your implementation, and treat these arguments to mean the same as the --includesub flag:

$ ./test.out [--include name,...] [--exclude name,...] [--includesub name,...] [--excludesub name,...] [word ...]

Flags:
--include Same as TestRunner::include(), implies initial TestRunner::exclude("*")
--exclude Same as TestRunner::exclude()
--includesub Include using substr(), implies initial TestRunner::exclude("*")
--excludesub Exclude using subtr()

Arguments:
[word ...] Same as [--includesub word,...]
bxparks commented 3 years ago

The nice thing about my flags is that they are incrementally backwards compatible with your implementation. So we don't have to implement everything at once. We can incrementally add each feature/flag. I just want to make sure that the existing TestRunner::include()/exclude() could be exposed as command line flags in a consistent and compatible way.

hsaturn commented 3 years ago

Hi Brian

I can agree with all what you said. I already have what I want with my local commit, and I'm 100% confident that your implementation will match all my needs.

I do agree that a kind of consistency with TestRunner inclusion/exclusion is good. I'll do the same if I were you. You may postpone this dev, my commit does the job for me right now.

Thanks for your concern

Best regards Francois Biot

Le mar. 21 sept. 2021 à 20:05, Brian Park @.***> a écrit :

The nice thing about my flags is that they are incrementally backwards compatible with your implementation. So we don't have to implement everything at once. We can incrementally add each feature/flag. I just want to make sure that the existing TestRunner::include()/exclude() could be exposed as command line flags in a consistent and compatible way.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bxparks/AUnit/issues/76#issuecomment-924231432, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADRCSGYQJ5VHZGPPXBOROWDUDDCOTANCNFSM5EKJIWXA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

bxparks commented 3 years ago

Can you sync up with my develop branch and try out the new command line options? The following flags and arguments are implemented:

$ ./CompareTest.out --help
Usage: ./CompareTest.out [--help|-h]
   [--include pattern,...] [--exclude pattern,...]
   [--includesub substring,...] [--excludesub substring,...]
   [--] [substring ...]
bxparks commented 3 years ago

The documentation is at https://github.com/bxparks/AUnit#CommandLineFlagsAndArguments

bxparks commented 3 years ago

Fixed with v1.6.0.