Fix unhandled error in actions_artifacts.go

[mo1ein]

No description provided.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gmlewis]

Thank you, @mo1ein !
Could you please add a unit test that exercises this new code path?

[gmlewis]

It also looks like you need to run gofmt on the edited file to fix the formatting. Please see our CONTRIBUTING.md guide for more information.

[codecov]

Codecov Report

Merging #2460 (1ac8032) into master (e2f7379) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2460   +/-   ##
=======================================
  Coverage   98.03%   98.03%           
=======================================
  Files         122      122           
  Lines       10689    10692    +3     
=======================================
+ Hits        10479    10482    +3     
  Misses        144      144           
  Partials       66       66           

Impacted Files	Coverage Δ
github/actions_artifacts.go	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mo1ein]

It also looks like you need to run gofmt on the edited file to fix the formatting. Please see our CONTRIBUTING.md guide for more information.

Always after writing code, I run gofmt but for this file, I can't see 8 spaces after gofmt. That's the reason of my second commit.

[gmlewis]

It also looks like you need to run gofmt on the edited file to fix the formatting. Please see our CONTRIBUTING.md guide for more information.

Always after writing code, I run gofmt but for this file, I can't see 8 spaces after gofmt. That's the reason of my second commit.

gofmt uses tab characters and not spaces. Maybe that explains the difference?

[mo1ein]

It also looks like you need to run gofmt on the edited file to fix the formatting. Please see our CONTRIBUTING.md guide for more information.

Always after writing code, I run gofmt but for this file, I can't see 8 spaces after gofmt. That's the reason of my second commit.

gofmt uses tab characters and not spaces. Maybe that explains the difference?
OMG! yeah. Thank's for your clarifying.

[gmlewis]

yeah. Thank's for your clarifying.

Cool!

Could you please add a unit test that exercises this new code path?

[mo1ein]

yeah. Thank's for your clarifying.

Cool!

Could you please add a unit test that exercises this new code path?

I can't figure out how to fix Added lines #L125 - L126 were not covered by tests. Can you help?

[gmlewis]

I can't figure out how to fix Added lines #L125 - L126 were not covered by tests. Can you help?

I believe that if you give a "Location" value that url.Parse will return an error for, then that should test the line. Maybe something like "https://\n/bogus\nurl" or something like that?

[mo1ein]

I can't figure out how to fix Added lines #L125 - L126 were not covered by tests. Can you help?

I believe that if you give a "Location" value that url.Parse will return an error for, then that should test the line. Maybe something like "https://\n/bogus\nurl" or something like that?

I added the test but it has a bug. I think the test idea is correct. Just need to produce URL parse error. I believe your review is helpful to fix bugs.

[gmlewis]

I added the test but it has a bug. I think the test idea is correct. Just need to produce URL parse error. I believe your review is helpful to fix bugs.

Apparently, according to the source code for url.Parse, if you put control characters in your URL, this function should return an error:

https://cs.opensource.google/go/go/+/refs/tags/go1.19.1:src/net/url/url.go;l=503-505;drc=d36466fe2aaf51c4b38c3a2ea3164cb3a56b8059

[mo1ein]

• Fixed. Everything is OK. :-)

[gmlewis]

Thank you, @mo1ein !
LGTM.
Merging.