storeapi: improve the error message for the case the Store answers an upload needs manual review.

[facundobatista]

LP: #1670471

[elopio]

I like the message a lot more now. Thanks @facundobatista.

What I don't like is that no test failed. We are missing tests here. I know this bumps a lot the time you assigned for this task, but could you work on the tests?

[elopio]

now the forum should be used, instead of the mailing list.

[facundobatista]

Thanks @elopio !

There I changed the text to mention the forum, not the mailing list.

Regarding the test case, I don' t think there is value in providing a test for just a simple text (with "simple" meaning that the text is not built or constructed in any way) as we have in this case.

Imagine how the test for this constant would be, just the text repeated in the code and the test, and if a change needed, the change is reproduced in both places, no really asserting nothing.

IMO constants are not to be tested, is like testing specific values of settings or configs.

Thanks again!

[codecov-io]

Codecov Report

Merging #1261 into master will decrease coverage by 1.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1261      +/-   ##
==========================================
- Coverage   96.38%   95.35%   -1.04%     
==========================================
  Files         194      218      +24     
  Lines       17811    20252    +2441     
  Branches     1370     1611     +241     
==========================================
+ Hits        17168    19312    +2144     
- Misses        435      667     +232     
- Partials      208      273      +65

Impacted Files	Coverage Δ
snapcraft/storeapi/errors.py	88.77% <ø> (+1.05%)	⬆️
snapcraft/__init__.py	73.33% <0%> (-26.67%)	⬇️
snapcraft/tests/fixture_setup.py	70.73% <0%> (-16.64%)	⬇️
snapcraft/tests/sources/test_mercurial.py	91.26% <0%> (-8.74%)	⬇️
snapcraft/internal/cache/_snap.py	84.12% <0%> (-7.88%)	⬇️
snapcraft/sources.py	92.3% <0%> (-7.7%)	⬇️
snapcraft/_options.py	82.29% <0%> (-7.37%)	⬇️
snapcraft/internal/lxd.py	93.27% <0%> (-6.73%)	⬇️
snapcraft/internal/sources/_deb.py	90% <0%> (-6.3%)	⬇️
snapcraft/plugins/kbuild.py	94.73% <0%> (-5.27%)	⬇️
... and 122 more

---

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 1a936f4...ce02ac7. Read the comment docs.

[elopio]

@facundobatista I agree more or less about the text. At some point I would love to have tests specifying the UX, instead of google docs, but we are not close to that yet. As we are usually at least checking parts of the message, I thought that we didn't even have a test for the exception. But I checked and we have test_upload_requires_review. All good, +1