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

tests: Reenable testAddAndRemoveData #2533

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

kevinrenskers
Copy link
Contributor

@kevinrenskers kevinrenskers commented Dec 14, 2022

I initially thought the test had failed twice, but it was actually only once. And the test failure wasn't a real test failure I think:

Failing tests:
	SentryTests:
		SentrySpanTests.testAddAndRemoveData()

But absolutely nowhere is there any information on which assert failed, and how. But since the test only uses XCTAssertEqual and XCTAssertNil, we should get proper failure messages.

So I don't know why the test failed in https://github.com/getsentry/sentry-cocoa/actions/runs/3530734813/jobs/5923080490, but I don't think it's because the actual test function is flaky.

#skip-changelog

@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Merging #2533 (e5ae92f) into 8.0.0 (53e149f) will decrease coverage by 0.12%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##            8.0.0    #2533      +/-   ##
==========================================
- Coverage   78.92%   78.79%   -0.13%     
==========================================
  Files         241      241              
  Lines       22118    22129      +11     
  Branches     9767     9775       +8     
==========================================
- Hits        17457    17437      -20     
- Misses       4217     4243      +26     
- Partials      444      449       +5     
Impacted Files Coverage Δ
Sources/Sentry/SentryProfilingLogging.mm 0.00% <0.00%> (-51.86%) ⬇️
Sources/Sentry/SentryTime.mm 57.89% <0.00%> (-31.58%) ⬇️
Sources/Sentry/include/SentryProfilingLogging.hpp 45.45% <0.00%> (-9.10%) ⬇️
Sources/Sentry/SentryThreadMetadataCache.cpp 91.30% <0.00%> (-4.35%) ⬇️
Sources/Sentry/SentryThreadHandle.cpp 69.39% <0.00%> (-3.28%) ⬇️
Sources/Sentry/SentryBacktrace.cpp 88.39% <0.00%> (-1.79%) ⬇️
Sources/Sentry/SentryFileManager.m 95.00% <0.00%> (-1.05%) ⬇️
Sources/Sentry/NSDictionary+SentrySanitize.m 100.00% <0.00%> (ø)
Sources/Sentry/SentryNetworkTracker.m 95.39% <0.00%> (+0.04%) ⬆️
Sources/Sentry/SentryTracer.m 95.05% <0.00%> (+0.53%) ⬆️
... and 1 more

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 53e149f...e5ae92f. Read the comment docs.

@kevinrenskers
Copy link
Contributor Author

codecov/project Failing after 1s — 78.79% (-0.13%) compared to 53e149f

So by adding more tests the coverage went down? Doesn't make sense.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann
Copy link
Member

codecov/project Failing after 1s — 78.79% (-0.13%) compared to 53e149f

So by adding more tests the coverage went down? Doesn't make sense.

Yeah, I also don't get this. Sometimes it's just bogus.

@kevinrenskers kevinrenskers merged commit 2803b21 into 8.0.0 Dec 15, 2022
@kevinrenskers kevinrenskers deleted the tests/reenable-testAddAndRemoveData branch December 15, 2022 16:30
kevinrenskers added a commit that referenced this pull request Dec 16, 2022
* 8.0.0: (31 commits)
  tests: Reenable testAddAndRemoveData (#2533)
  feat: support SENTRY_DSN environment var on macOS (#2534)
  Remove duplicate entry (#2532)
  fix: ARC issue for FileManager (#2525)
  feat: Add SwiftUI performance tracking (#2271)
  fix: Remove all permission checks (#2529)
  Remove the automatic `viewAppearing` span (#2511)
  Fix and reenable testANRDetected_UpdatesAppStateToTrue (#2526)
  fix: Don't add out of date context for crashes (#2523)
  ref: Rename isOOM to watchdog in Client (#2520)
  test: Fix disabled failing watchdog test (#2521)
  build(deps): bump github/codeql-action from 2.1.35 to 2.1.36 (#2516)
  Rename the watchdog option and integration (#2513)
  feat: Enable CaptureFailedRequests by default (#2507)
  test: Fix asserts for SentryCrashTestInstallation (#2500)
  meta: User interaction tracing enabled per default (#2506)
  ref: Rename OOM to Watchdog Terminations (#2499)
  feat: enableUserInteractionTracing is GA (#2503)
  build(deps): bump nokogiri from 1.13.9 to 1.13.10 (#2505)
  perf: Don't attach headers for SentryNoOpSpan (#2498)
  ...
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