flow-storm / clofidence

Bolster your Clojure test suite confidence
The Unlicense
46 stars 2 forks source link

Improvements to filtering #1

Closed imrekoszo closed 12 months ago

imrekoszo commented 12 months ago

As per https://clojurians.slack.com/archives/C06MAR553/p1700151357158949?thread_ts=1700140077.982029&cid=C06MAR553

1. namespaces

Currently this project defines namespace allow- and block lists based on namespace prefixes. As far as I am aware, there is a convention in Clojure to suffix test namespaces with -test, and so the current blocklist definition doesn't allow me to quickly exclude all test namespaces from instrumentation/reporting.

2. forms

The readme for this project states:

Which forms are included in the report?

By default only forms with the first symbol name being one of : defn, defn-, defmethod, extend-type, extend-protocol, deftype and defrecord.

If you have other types of forms like the ones defined by some macros, you can include them by using :extra-forms in the configuration parameters. It takes a set of symbols like :extra-forms #{defroutes my-macro}.

As I noticed when I tried Clofidence, and as it was explained to me in the Slack thread linked above, instrumentation does happen on every form that passes the above allow and block lists, but only the ones mentioned in this part of the readme and the ones added to :extra-forms will actually show up in the report.

In a codebase that uses top-level macros heavily it might be inconvenient to have to add every new macro to this list. An alternative could be to report on all but a select few known instrumented forms by default and add an option for a blocklist so macros one really doesn't want to see in the report can be excluded.

It could also be considered to skip instrumenting (top-level) forms that are not to be reported on, but I know far too little about the project to be able to suggest whether that would be a good idea or even possible.

jpmonettas commented 12 months ago

Both of this were assessed and are part of the already released 0.2.0, which also needs ClojureStorm >= 1.11.1-15

For 1. you can now do like clojure.storm.instrumentSkipRegex=.*test.* to skip instrumenting namespaces that contains the word test.

For 2. there is now :block-forms as described in the Readme

The performance of 0.2.0 should also be much better, and bigger forms instrumentation should also be supported now.