-
Notifications
You must be signed in to change notification settings - Fork 129
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
Fix codecov and test failures #662
Conversation
541ef0a
to
0192719
Compare
0192719
to
d958342
Compare
See #667 |
Updated PR to fix two issues causing test and coverage failure:
|
af026a1
to
04e01dc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #662 +/- ##
========================================
Coverage 96.24% 96.24%
========================================
Files 41 41
Lines 5220 5220
Branches 1226 1226
========================================
Hits 5024 5024
Misses 124 124
Partials 72 72 ☔ View full report in Codecov by Sentry. |
0c56a27
to
04e01dc
Compare
Ugh, this changing viewbox parameter in the generated svg is killing me!! |
f6c823a
to
9e743a6
Compare
597f1f2
to
b17fad7
Compare
All working now! |
doorstop/core/tests/test_all.py
Outdated
@@ -575,6 +586,14 @@ def setUp(self): | |||
self.tree = core.build(cwd=FILES, root=FILES) | |||
self.document = self.tree.find_document("REQ") | |||
self.temp = tempfile.mkdtemp() | |||
# Use local plantuml of known version | |||
plantuml_cmd = "java -Dplantuml.include.path=includes -jar " + os.path.join( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to make java
a dependency of this library. Can you solve this with a Python library?
Hi @robtaylor, thanks for getting CI to pass! Can you open a separate pull request with just the code coverage fixes? As mentioned in the commend above, I don't think we want to add |
Hi Jace, yes for sure.
So with respect to plantuml, there isn't really any options - you need to
have java.
That said, as I'm now stripping out the SVG, maybe a better approach here
is to mock calling plantuml. What do you think?
…On Thu, 3 Oct 2024 at 10:57, Jace Browning ***@***.***> wrote:
Hi @robtaylor <https://github.com/robtaylor>, thanks for getting CI to
pass!
Can you open a separate pull request with just the code coverage fixes? As
mentioned in the commend above, I don't think we want to add java as a
dependency in order to get tests to pass. I'd like to review configuration
changes and business logic changes separately.
—
Reply to this email directly, view it on GitHub
<#662 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB4O4GZWVMUTZFUHVHFOWDZZVLNRAVCNFSM6AAAAABPCIPFDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJRGYZTSNJWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
9a86a45
to
263d98d
Compare
Hey @jacebrowning, I've put the code changes (plantuml mock) in #669 . This now has just the CI changes. Both patches are needed for successful CI - the draft PR #670 checks this! |
This should fix the issue with linux showing as failing.