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

Add some unit tests for config parsers where coverage was missing #3809

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

sparrc
Copy link
Contributor

@sparrc sparrc commented Jul 18, 2023

Summary

Some env var config parsing situations were missing coverage, adding unit tests for fuller coverage. Some unit test files were also missing the "unit" tag, which meant they were not running on PRs. Tags were added to these files.

before:

% go test -cover -tags unit ./config/.
ok      github.com/aws/amazon-ecs-agent/agent/config    0.460s  coverage: 91.0% of statements

after:

% go test -cover -tags unit ./config/.
ok      github.com/aws/amazon-ecs-agent/agent/config    0.480s    coverage: 92.6% of statements

Testing

n/a

Description for the changelog

no application code changes

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sparrc sparrc requested a review from a team as a code owner July 18, 2023 17:12
@sparrc sparrc force-pushed the unit-tests-for-envvar-parsers branch 2 times, most recently from 7f210b3 to 0696bc5 Compare July 18, 2023 18:17
before:
% go test -cover -tags unit ./config/.
ok      github.com/aws/amazon-ecs-agent/agent/config    0.460s  coverage: 91.0% of statements

after:
% go test -cover -tags unit ./config/.
ok      github.com/aws/amazon-ecs-agent/agent/config    0.480s    coverage: 92.6% of statements
@sparrc sparrc merged commit 26c64c0 into aws:dev Jul 20, 2023
6 checks passed
@sparrc sparrc deleted the unit-tests-for-envvar-parsers branch July 20, 2023 16:15
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.

4 participants