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.

Sign up for GitHub

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

Jump to bottom

beginner: add coverage #1963

Open
mattklein123 opened this issue Oct 28, 2017 · 10 comments
Open

beginner: add coverage #1963

mattklein123 opened this issue Oct 28, 2017 · 10 comments
Labels
beginner Good starter issues! help wanted Needs help! tech debt

Comments

@mattklein123
Copy link
Member

mattklein123 commented Oct 28, 2017 •
edited
Loading

Going to leave open a persistent beginner ticket for adding coverage:
https://s3.amazonaws.com/lyft-envoy/coverage/report-master/coverage.html

There are always a few missing things here and there especially regarding random error handling. If you are a beginner this is a great way to learn and do something valuable.

Rough steps:

  • Look at the coverage report and find something (often a single line) that looks like it should be covered.
  • Find the relevant unit test that should cover it (often in the test tree with the same file name)
  • Write the test
  • Confirm in PR coverage build that line is covered. (Coverage report is posted as a build artifact).
@mattklein123 mattklein123 added beginner Good starter issues! help wanted Needs help! tech debt labels Oct 28, 2017
@minhlongdo
Copy link

@mattklein123 Hi, could I pick this one up?

@mattklein123
Copy link
Member Author

@minhlongdo this is a "perma issue." Feel free to just go ahead and add some coverage!

@andrzejwaw andrzejwaw mentioned this issue Jun 7, 2019
Merged
andrzejwaw added a commit to andrzejwaw/envoy that referenced this issue Jun 7, 2019
@andrzejwaw
Description: Added coverage of methods translateVirtualCluster and tr…
059d411 
…anslateCors to RdsJson class

Risk Level: Low

Testing: unit test

Release Notes: N/A

Fixes #Issue: envoyproxy#1963

Signed-off-by: Andrzej Wawrzyniak <andrzejwaw@gmail.com>
@williambanfield
Copy link

@mattklein123 I went to try my hand at one of these and noticed that the s3 bucket gives an AccessDenied error. Any idea why that may be?

@mattklein123
Copy link
Member Author

Sorry the coverage report has moved here: https://storage.googleapis.com/envoy-coverage/report-master/index.html

@saiskee
Copy link
Contributor

saiskee commented Apr 10, 2021 •
edited
Loading

@mattklein123 Where can I find the coverage report? Above link gives key does not exist error

Edit: Found it at https://storage.googleapis.com/envoy-postsubmit/main/coverage/index.html

@phlax
Copy link
Member

phlax commented Apr 10, 2021

@saiskee clicking that link work for me - not sure what the error you saw was but should work

@saiskee
Copy link
Contributor

saiskee commented Apr 10, 2021

@phlax
image

@phlax
Copy link
Member

phlax commented Apr 10, 2021 •
edited
Loading

apologies - i see now - i clicked the link that you posted - the one that works

@saiskee saiskee mentioned this issue Apr 10, 2021
Merged
@tommyp1ckles tommyp1ckles mentioned this issue Dec 16, 2021
Merged
htuch pushed a commit that referenced this issue Dec 19, 2021
@tommyp1ckles
Add test coverage for json package (#19300)
5e77408 
Adding more tests to increase line coverage for json package. Adding coverage for missing code paths by testing various loadFromString inputs.

Risk Level: Low
Testing: Added tests to json_loader_test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

This PR seeks to improve this issue: #1963

Signed-off-by: Tom Hadlaw <tomhadlaw@gmail.com>
joshperry pushed a commit to joshperry/envoy that referenced this issue Feb 13, 2022
@tommyp1ckles
Add test coverage for json package (envoyproxy#19300)
db35702 
Adding more tests to increase line coverage for json package. Adding coverage for missing code paths by testing various loadFromString inputs.

Risk Level: Low
Testing: Added tests to json_loader_test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

This PR seeks to improve this issue: envoyproxy#1963

Signed-off-by: Tom Hadlaw <tomhadlaw@gmail.com>
Signed-off-by: Josh Perry <josh.perry@mx.com>
@JayLog22
Copy link

Hi! I would love to help a bit with coverage, but getting access denied on the link above for coverage report. Is help still needed?

@mattklein123
Copy link
Member Author

https://storage.googleapis.com/envoy-postsubmit/main/coverage/index.html should be the working URL and I think it's wide open.

jpsim pushed a commit that referenced this issue Nov 28, 2022
@carloseltuerto @jpsim
Adapt Cronvoy to use the onComplete callback to get the EnvoyFinalStr…
3045bfd 
…eamIntel. (#1963)

Initial integration on the FinalStreamIntel for Cronvoy
Java tests added for Envoy-Mobile and Cronvoy
Description: Adapt Cronvoy to onComplete callBack
Risk Level: Small
Testing: Tests were added for Envoy-Mobile and Cronvoy
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this issue Nov 29, 2022
@carloseltuerto @jpsim
Adapt Cronvoy to use the onComplete callback to get the EnvoyFinalStr…
8637b45 
…eamIntel. (#1963)

Initial integration on the FinalStreamIntel for Cronvoy
Java tests added for Envoy-Mobile and Cronvoy
Description: Adapt Cronvoy to onComplete callBack
Risk Level: Small
Testing: Tests were added for Envoy-Mobile and Cronvoy
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: JP Simard <jp@jpsim.com>
simplyatul added a commit to simplyatul/envoy that referenced this issue Jun 26, 2023
@simplyatul
test coverage added for LowerCaseString class
beeaf5c 
Risk Level: Low
Testing: a Test added to header_map_impl_test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

This PR seeks to improve this issue: envoyproxy#1963

Signed-off-by: Atul Thosar <atulthosar@gmail.com>
@simplyatul simplyatul mentioned this issue Jun 26, 2023
Merged
yanavlasov pushed a commit that referenced this issue Jun 26, 2023
@simplyatul
test coverage added for LowerCaseString class (#28141)
e907eca 
This PR seeks to improve this issue: #1963

Signed-off-by: Atul Thosar <atulthosar@gmail.com>
asheryerm pushed a commit to asheryerm/envoy that referenced this issue Jul 5, 2023
@simplyatul @asheryerm
test coverage added for LowerCaseString class (envoyproxy#28141)
372da7b 
This PR seeks to improve this issue: envoyproxy#1963

Signed-off-by: Atul Thosar <atulthosar@gmail.com>
Signed-off-by: asheryer <asheryer@amazon.com>
reskin89 pushed a commit to reskin89/envoy that referenced this issue Jul 11, 2023
@simplyatul @reskin89
test coverage added for LowerCaseString class (envoyproxy#28141)
f6f9aaa 
This PR seeks to improve this issue: envoyproxy#1963

Signed-off-by: Atul Thosar <atulthosar@gmail.com>
Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
beginner Good starter issues! help wanted Needs help! tech debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@phlax @saiskee @minhlongdo @williambanfield @mattklein123 @JayLog22