Skip to content

Get codecov working again#67

Merged
dfed merged 1 commit into
mainfrom
dfed--codecov
Oct 11, 2022
Merged

Get codecov working again#67
dfed merged 1 commit into
mainfrom
dfed--codecov

Conversation

@dfed

@dfed dfed commented Oct 11, 2022

Copy link
Copy Markdown
Owner

PRs are no longer getting code coverage reports. It looks like we stopped successfully uploading code coverage reports when we moved to Github Actions:
image

I'm not sure how that PR broke things, but running locally I see that test runs utilizing swift packages have a .framework suffix, and test runs utilizing a generated xcodeproj do not. Fixing this seems to have fixed codecov.

@codecov

codecov Bot commented Oct 11, 2022

Copy link
Copy Markdown

Codecov Report

Merging #67 (8920426) into main (aa680f4) will increase coverage by 96.14%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           main      #67       +/-   ##
=========================================
+ Coverage      0   96.14%   +96.14%     
=========================================
  Files         0       26       +26     
  Lines         0     1996     +1996     
=========================================
+ Hits          0     1919     +1919     
- Misses        0       77       +77     
Impacted Files Coverage Δ
Sources/SwiftTryCatch/SwiftTryCatch.m 84.61% <0.00%> (ø)
Tests/SwiftTryCatchTests/SwiftTryCatchTests.swift 95.74% <0.00%> (ø)
Sources/CacheAdvance/FileHandleExtensions.swift 57.14% <0.00%> (ø)
Sources/CacheAdvance/BigEndianHostSwappable.swift 100.00% <0.00%> (ø)
Sources/CacheAdvance/BoolExtensions.swift 100.00% <0.00%> (ø)
...sts/CacheAdvanceTests/CacheHeaderHandleTests.swift 100.00% <0.00%> (ø)
Tests/CacheAdvanceTests/TestableMessage.swift 100.00% <0.00%> (ø)
Sources/CADCacheAdvance/CADCacheAdvance.swift 70.45% <0.00%> (ø)
Sources/CacheAdvance/CacheReader.swift 93.15% <0.00%> (ø)
...s/CacheAdvance/UInt64+BigEndianHostSwappable.swift 100.00% <0.00%> (ø)
... and 16 more

@dfed dfed marked this pull request as ready for review October 11, 2022 19:44
@dfed dfed requested review from bachand and fdiaz October 11, 2022 19:45
@dfed

dfed commented Oct 11, 2022

Copy link
Copy Markdown
Owner Author

This change works, and only affects (improves!) CI. I'm going to merge it prior to review.

@dfed dfed merged commit 2bfee6c into main Oct 11, 2022
@dfed dfed deleted the dfed--codecov branch October 11, 2022 19:45
@dfed

dfed commented Oct 11, 2022

Copy link
Copy Markdown
Owner Author

So, while this is getting us some coverage information, this fix does not seem to be working terribly well.

Here's the run for this job on main: https://github.com/dfed/CacheAdvance/actions/runs/3229766624
This job shows "no coverage report found" on every "Upload Coverage Report" job.

However, there is a coverage job for this merge commit on main: https://app.codecov.io/gh/dfed/CacheAdvance/commit/2bfee6c0f668735368bf8337cd726928002b1194

Weirder yet, I'm not seeing coverage information for other branches that branched from this sha.

@bachand

bachand commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator

I'm not sure how that PR broke things, but running locally I see that test runs utilizing swift packages have a .framework suffix, and test runs utilizing a generated xcodeproj do not. Fixing this seems to have fixed codecov.

Can you help me connect how this ^ comment relates to the change we made in #53.

In that PR it seems like we're still testing via the generated Xcode project:

"-project", "generated/CacheAdvance.xcodeproj",

@dfed dfed mentioned this pull request Oct 12, 2022
@dfed

dfed commented Oct 12, 2022

Copy link
Copy Markdown
Owner Author

Honestly, I have no idea how #53 broke us. And unfortunately the CI logs are no longer available that far back. I'm trying to fix forward with #68 but may have to try to reverse engineer what went wrong.

@dfed

dfed commented Oct 12, 2022

Copy link
Copy Markdown
Owner Author

Note that Valet is still working, which has a similar config (see square/Valet#256). So... at least we know something that works.

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