Add POST handler tests for cloning

[mpacer]

This continues #102 to add more testing for the post handler.

I have not yet gotten the mock to work correctly to test the newly introduced helper function.

This also adds a quick fix to the js files that had broken the CI tests when run locally. It's concerning to me that that didn't come up in our CI testing, since updating a const should always cause an issue which suggests the tests weren't being run

[mpacer]

@MSeal Is the credentials stuff the kind of thing I should try to mock with moto?

@willingc should we choose to test the python on only one of circleci or travis? Is there an advantage to testing it in both places?

[willingc]

@willingc should we choose to test the python on only one of circleci or travis? Is there an advantage to testing it in both places?

For now, let's keep both. They are a good check against each other while we develop the tests.

[willingc]

@mpacer Can we split out the quick fix for the CI from this PR? This way we have valid js tests again and we can iterate on the post tests.

[willingc]

@mpacer Can you rebase this on the latest master? I'm running the python tests locally and they are passing where Travis has a couple failing. 🤷‍♀

[mpacer]

@mpacer Can you rebase this on the latest master? I'm running the python tests locally and they are passing where Travis has a couple failing. 🤷‍♀

It was already rebased on the latest master — this has to do with how s3 configuration is set up. My guess is you have an ~/.aws/credentials file and so this isn't failing in the same way :/

this is why I think we need to use moto to mock the underlying call.

[mpacer]

@mpacer Can we split out the quick fix for the CI from this PR? This way we have valid js tests again and we can iterate on the post tests.

Done with #107

[willingc]

Let's do this..rebase this PR. If the tests don't pass, remove the two tests where the mocking is not working fully. Then move those two tests to a new PR and we can merge this.

[mpacer]

I think at this point I might want to rebase it on #121 since those tests will need to be on their own separate handler anyway, otherwise we're going to run into a conflict with that one where we're trying to test functions that no longer exist.

[codecov[bot]]

Codecov Report

Merging #106 into master will decrease coverage by 4.77%. The diff coverage is 34.61%.

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   34.72%   29.94%   -4.78%     
==========================================
  Files          11       11              
  Lines         360      374      +14     
==========================================
- Hits          125      112      -13     
- Misses        235      262      +27

[codecov[bot]]

Codecov Report

Merging #106 into master will increase coverage by 1.9%. The diff coverage is 34.61%.

@@            Coverage Diff            @@
##           master     #106     +/-   ##
=========================================
+ Coverage   34.72%   36.63%   +1.9%     
=========================================
  Files          11       11             
  Lines         360      374     +14     
=========================================
+ Hits          125      137     +12     
- Misses        235      237      +2

[mpacer]

Well… that is a disappointing code_coverage report. I suppose that's mostly because of the refactor & the fact that without figuring out the moto stuff the tests going to exit pretty early in the method.

[mpacer]

I don't know why the commits are appearing out of order, but the add mock_s3 commit is supposed to come before the other two, which is why it seems to be testing not the latest commit… something weird is happening there… not really sure.

[mpacer]

They're still out of order, but I think this should improve the coverage.

[mpacer]

Does anyone else know why CodeCov isn't picking up the latest version of the changes that a bunch more of the publish handler?

[willingc]

I think that I configured codecov to only do master. Let's land this. We can fiddle with the dials later.