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

#704 - Enforce failure reasons in tests #899

Merged
merged 1 commit into from
May 29, 2018
Merged

Conversation

neonailol
Copy link
Contributor

@neonailol neonailol commented May 22, 2018

Fix For #704
Same as PR #744 but only prints errors without failing the build

@0crat 0crat added the scope label May 22, 2018
@0crat
Copy link
Collaborator

0crat commented May 22, 2018

Job #899 is now in scope, role is REV

@codecov-io
Copy link

codecov-io commented May 22, 2018

Codecov Report

Merging #899 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #899   +/-   ##
=========================================
  Coverage     87.09%   87.09%           
  Complexity     1457     1457           
=========================================
  Files           259      259           
  Lines          3773     3773           
  Branches        212      212           
=========================================
  Hits           3286     3286           
  Misses          439      439           
  Partials         48       48

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1e27f3...b421f88. Read the comment docs.

@llorllale
Copy link
Contributor

@neonailol please use the maven configuration merged in #882.

@neonailol
Copy link
Contributor Author

@llorllale updated

pom.xml Outdated
@@ -156,7 +156,7 @@ The MIT License (MIT)
-->
<failOnViolation>false</failOnViolation>
<signaturesFiles>
<signaturesFile>./src/test/resources/forbidden-apis.txt</signaturesFile>
<signaturesFile>./src/verifier/forbiddenapis.txt</signaturesFile>
Copy link
Contributor

Choose a reason for hiding this comment

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

@neonailol why not use ./src/test/resources/forbidden-apis.txt like in #882

Copy link
Contributor Author

@neonailol neonailol May 28, 2018

Choose a reason for hiding this comment

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

@llorllale Because it is not part of sources, just like verifications.xml, and should not be in classpath

Copy link
Contributor

Choose a reason for hiding this comment

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

@neonailol I don't understand. src/test/resources is exactly the path for these kinds of things. It won't be exported to production when released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@llorllale well src/test/resources as stated from the path, is the directory for resources used by tests, this, on the other hand, is the resources needed to perform static verification of all sources. I expect in the future someone will be using this to prohibit uses of some other methods in library code, not just the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

@neonailol we'll do that when the need arises. For now, please put the file back in src/test/resources, thanks

@llorllale
Copy link
Contributor

@neonailol see my comment above

@neonailol
Copy link
Contributor Author

@llorllale done, it is also worth to check out bundled with the plugin signatures

@llorllale
Copy link
Contributor

@neonailol good to know, thanks

@llorllale
Copy link
Contributor

@rultor merge

@rultor
Copy link
Collaborator

rultor commented May 29, 2018

@rultor merge

@llorllale OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit b421f88 into yegor256:master May 29, 2018
@rultor
Copy link
Collaborator

rultor commented May 29, 2018

@rultor merge

@llorllale Done! FYI, the full log is here (took me 10min)

@0crat 0crat removed the scope label May 30, 2018
@0crat
Copy link
Collaborator

0crat commented May 30, 2018

The job #899 is now out of scope

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.

5 participants