Skip to content

Add compliance test and annotate existing ones#669

Merged
brettmc merged 7 commits into
open-telemetry:mainfrom
yuktea:compliance-test-and-annotations
May 6, 2022
Merged

Add compliance test and annotate existing ones#669
brettmc merged 7 commits into
open-telemetry:mainfrom
yuktea:compliance-test-and-annotations

Conversation

@yuktea

@yuktea yuktea commented May 3, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@yuktea
yuktea force-pushed the compliance-test-and-annotations branch 3 times, most recently from c0bcfb3 to 0820860 Compare May 5, 2022 19:01
@codecov

codecov Bot commented May 5, 2022

Copy link
Copy Markdown

Codecov Report

Merging #669 (a9270a4) into main (4592a27) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #669      +/-   ##
============================================
+ Coverage     85.26%   85.28%   +0.02%     
+ Complexity     1153     1151       -2     
============================================
  Files           128      128              
  Lines          2789     2794       +5     
============================================
+ Hits           2378     2383       +5     
  Misses          411      411              
Flag Coverage Δ
7.4 85.24% <100.00%> (+0.02%) ⬆️
8.0 85.28% <100.00%> (+0.02%) ⬆️
8.1 85.28% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/SDK/Trace/Span.php 93.91% <100.00%> (+0.21%) ⬆️

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 4592a27...a9270a4. Read the comment docs.

@yuktea
yuktea marked this pull request as ready for review May 5, 2022 19:03
@yuktea

yuktea commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

All tests passing here. We could merge this first and rebase #670 before review.

Comment thread tests/Unit/SDK/Trace/TracerProviderTest.php Outdated
Comment thread tests/Unit/SDK/Trace/TracerProviderTest.php Outdated
Comment thread tests/Unit/SDK/Trace/TracerProviderTest.php Outdated
Comment thread tests/Unit/SDK/Trace/SpanTest.php Outdated
$events = $span->toSpanData()->getEvents();
$c = 0;

$this->assertEvent($events[$c++], 'a', new Attributes(), self::START_EPOCH);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this would be more readable if you just provided integer values.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed! I've pushed a9270a4 to improve readability here. Can you take a look?

@yuktea
yuktea force-pushed the compliance-test-and-annotations branch from 8d3b6c4 to a9270a4 Compare May 6, 2022 09:42
@brettmc
brettmc merged commit 8ae46f2 into open-telemetry:main May 6, 2022
@yuktea
yuktea deleted the compliance-test-and-annotations branch June 2, 2022 02:44
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