win_get_url - Make checksum work more like ansible.builtin.get_url

[KBjorndal-VizRT]

SUMMARY

Makes checksum handling behave more like ansible.builtin.get_url

Fixes #717

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

win_get_url

ADDITIONAL INFORMATION

I'm not 100% sure about the logic to trigger collecting $dest_checksum, it is technically called in some circumstances it wasn't before, though if I read the code right that was only in error cases since all normal exits would calculate it at the end of the script.

[jborean93]

The changes you've added here look good. Appreciate you updating the docs and adding the changelog fragment but are you also able to add a test for this specific scenario. We currently have some tests with the checksum option under https://github.com/ansible-collections/ansible.windows/blob/main/tests/integration/targets/win_get_url/tasks/tests_checksum.yml but it doesn't look like there are any ones for idempotency checking when force: false is set.

If you need help with any of this please let me know and I can guide you where I can.

[KBjorndal-VizRT]

The changes you've added here look good. Appreciate you updating the docs and adding the changelog fragment but are you also able to add a test for this specific scenario. We currently have some tests with the checksum option under https://github.com/ansible-collections/ansible.windows/blob/main/tests/integration/targets/win_get_url/tasks/tests_checksum.yml but it doesn't look like there are any ones for idempotency checking when force: false is set.

I added a test for the behaviour where it shouldn't do any download if checksum already matches.

Tried a few different ways but can't think of how to make an automated test for the other part. As far as I can see httpbin only sends 304 replies for the /cache endpoint, which otherwise returns dynamic data so it's impossible to make it emulate the case where we have a file that differs by checksum but the If-Modified-Since logic would say there is nothing new at the URL.

[KBjorndal-VizRT]

Not sure what the failing check here is about, is it related to the PR or some CI infrastructure?

[jborean93]

I added a test for the behaviour where it shouldn't do any download if checksum already matches.

Thanks, appreciate it.

Tried a few different ways but can't think of how to make an automated test for the other part. As far as I can see httpbin only sends 304 replies for the /cache endpoint, which otherwise returns dynamic data so it's impossible to make it emulate the case where we have a file that differs by checksum but the If-Modified-Since logic would say there is nothing new at the URL.

I think that's fine to ignore for now, if we get any future regressions we can look at potentially spinning up a custom HTTP endpoint just as a test but I think that's overkill for now.

Not sure what the failing check here is about, is it related to the PR or some CI infrastructure?

There seems to be some trouble with Zuul so the failure is unrelated here.