Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to specify patterns to be excluded from coverage report #6

Merged
merged 4 commits into from
Feb 26, 2016

Conversation

scottclowe
Copy link
Contributor

Closes #4.

Adds -cover_exclude option, which can be to specify patterns for directories and files which should be excluded from the coverage report. This can be used multiple times to specify multiple patterns.

The implementation method was as discussed on the thread for #4.

I have not altered README.md, so there is currently no documentation for this feature.

Will exclude matching directories and files from the list.
This adds an option to the API to specify patterns which should not
be included in the coverage report.

Closes MOxUnit#4.
@@ -67,7 +86,9 @@
if ~ignore_fn
path_fn=fullfile(root_dir, fn);

if isdir(path_fn)
if ~isempty(regexp(fn,exclude_re,'once'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this would also exclude directories that match the pattern. Is that intended, or would it make more sense to limit this to files (not directories) only? In any case, can you please add this in the documentation (around line 15)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 074542d.

@nno
Copy link
Contributor

nno commented Feb 25, 2016

Thanks. this looks very good. I added two minor comments.
What are your thoughts about limiting this to files only, or to both files and directory?

@nno
Copy link
Contributor

nno commented Feb 26, 2016

Thanks @scottclowe, looks great. I will wait for travis to finish; if it is happy, I will merge.

@scottclowe
Copy link
Contributor Author

I fixed the two issues you picked up on. Thanks for pointing them out!

I would like to be able to omit both files and directories. We could have two arguments, one which files must match and another for directories but I think this is cleaner.

There are two use-cases which this fix will improve

  1. The problem when a script/function, such as getPackagePath.m discussed on Add option to modify files for coverage tracking in-place #5, needs to be kept in the same directory otherwise the code will break. This can now be omitted from coverage and so it will be left in the same place.
  2. When test functions or external code is in the same top-level folder as the code which needs to be tested, or the code base has multiple modules and each module has its own test folder within the module folder.

nno added a commit that referenced this pull request Feb 26, 2016
NF: Add ability to specify patterns to be excluded from coverage report
@nno nno merged commit 41ae7db into MOxUnit:master Feb 26, 2016
@scottclowe
Copy link
Contributor Author

Support for this API change still needs to be implemented in MOxUnit/MOxUnit.

I can do that too if you'd like?

scottclowe added a commit to scottclowe/MOxUnit that referenced this pull request Feb 26, 2016
@nno
Copy link
Contributor

nno commented Feb 26, 2016

@scottclowe, thanks for the offer; I would appreciate it if you could implement that in MOxUnit and send a PR.

@nno
Copy link
Contributor

nno commented Feb 26, 2016

@scottclowe sent out a PR at MOxUnit/MOxUnit#22

scottclowe added a commit to scottclowe/MOxUnit that referenced this pull request Feb 26, 2016
scottclowe added a commit to scottclowe/MOxUnit that referenced this pull request Feb 26, 2016
scottclowe added a commit to scottclowe/MOxUnit that referenced this pull request Feb 26, 2016
wahln pushed a commit to e0404/MOcov that referenced this pull request Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants