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 flag --fail-on-uncovered (closes #306) #307

Merged
merged 3 commits into from
Jun 5, 2020
Merged

Add flag --fail-on-uncovered (closes #306) #307

merged 3 commits into from
Jun 5, 2020

Conversation

hugochinchilla
Copy link
Contributor

Here is a patch to allow failure if uncovered cases are found.

I could not find any test to cover the changes made to the command nor the Context object, if tests are mandatory please explain where should I fit a test for this use-case.

@smoench
Copy link
Contributor

smoench commented Jun 2, 2020

@hugochinchilla Thank you, nice idea!
This might be covered by a functional test. I already started some in https://github.com/sensiolabs-de/deptrac/blob/master/.github/workflows/continuous-integration.yml#L124 maybe there is a better solution this.

Currently internal php classes are also blamed as uncovered. Need to fix this first.

@hugochinchilla
Copy link
Contributor Author

Currently internal php classes are also blamed as uncovered. Need to fix this first.

For me this is not a stopper as I have defined a layer named Language where I define all the internal classes to keep uncovered cases at zero.

This is an example of what I do:

    -   name: Language
        collectors:
            -   type: className
                regex: ^Exception$
            -   type: className
                regex: ^JsonException$
            -   type: className
                regex: ^Throwable$

@smoench smoench added this to the 0.8 milestone Jun 4, 2020
@hugochinchilla
Copy link
Contributor Author

@smoench I have updated the PR to include new logic to not blame php internal classes as uncovered. There are unit tests for this feature.

I still can't make a test for the --fail-on-uncovered flag as I don't know how could a introduce a step in the github actions which is expected to fail to mark the step as succeeded. I could do it by executing a shell script which does the test and expects the exit status to be non-zero but seems unclean to me, if this is ok to you tell me where should I place such script.

@smoench
Copy link
Contributor

smoench commented Jun 4, 2020

@hugochinchilla Thanks for try figuring out how to resolve internal class likes. I think the best idea would to resolve this in an other PR.

I still can't make a test for the --fail-on-uncovered flag as I don't know how could a introduce a step in the github actions which is expected to fail to mark the step as succeeded. I could do it by executing a shell script which does the test and expects the exit status to be non-zero but seems unclean to me, if this is ok to you tell me where should I place such script.

This file could be added in .github/workflows.

@hugochinchilla
Copy link
Contributor Author

@smoench I have created #310 for the other changes and removed them from this PR.

I have also added the missing test for this feature.

@smoench
Copy link
Contributor

smoench commented Jun 5, 2020

Thank you @hugochinchilla

@smoench smoench merged commit 6dfc004 into qossmic:master Jun 5, 2020
@hugochinchilla hugochinchilla deleted the fail-on-uncovered branch June 5, 2020 07:59
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