-
Notifications
You must be signed in to change notification settings - Fork 22
Implement Unit Test for AWS Metric Attributes Span Exporter #30
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
Implement Unit Test for AWS Metric Attributes Span Exporter #30
Conversation
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Show resolved
Hide resolved
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.
Please go over the file:
- add type hint for all variables when it is first initialized.
- add all comments the same as Java tests.
- There should be no module level function, please move them into the class and mark as internal use starting with
_
. Same applies for variables.
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.
Please add one more test to address this :
Line 105 in 3c822a8
# TODO: AwsMetricAttributesSpanExporter depends on internal ReadableSpan method _attributes. |
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
...lemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py
Outdated
Show resolved
Hide resolved
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 only add comments for partial of the files. Please go over the file carefully, and add the type hint for all variable initialization that is missing .
…tes_span_exporter' into unittest_test_aws_metric_attributes_span_exporter
)" This reverts commit 928a5be.
Description of changes:
Implement Unit Test for AWS Metric Attributes Span Exporter
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.