chocolatey / chocolatey-ansible

The Chocolatey module collection for Ansible
GNU General Public License v3.0
49 stars 30 forks source link

(#122) add version gates and split tests for choco v1 #121

Closed vexx32 closed 1 year ago

vexx32 commented 1 year ago

Description Of Changes

Motivation and Context

Unsure if this is the way we want to go yet, but at least this is a decently clear illustration of what one option is here.

Testing

  1. Ran the tests locally
  2. Running in CI

Operating Systems Testing

N/A

Change Types Made

Change Checklist

Related Issue

Fixes #122

Windos commented 1 year ago

The failed_when: not allow_multiple_cli_v2.failed was doing the job of supressing the expected failure, but meant that the assertion after than was failing (because failed_when was flipping the failure to a success)

I've pushed a commit that swaps out the failed_when for simply ignoring errors, meaning the expected failure won't stop the playbook executing and the assertion can still test for the failure.

This has worked, in so far as the testing for that expected failure now passes, however it flows into other things that'll need to be in the v1 testing, specifically explicitly pinning an older version when two version are installed.

Windos commented 1 year ago

Mentioning this here as I'm not sure if we want to cover the syntax for v1.x under this PR or if I should split this off into its own issue/PR, but there are a number of instances of choco list --local-only in the tests.

I count 20 under win_chocolatey/tasks/tests.yml

There's also the decision around what to do with the module utils Get-ChocolateyPackage function, as it's a hard coded argument on the list command there.

vexx32 commented 1 year ago

Hmm, all good points, thanks for pointing those out 🤔

It would probably be simplest to do a breaking release at this point where v1.x of the collection supports CLI 1.x fully and v2.x of the collection supports CLI 2.x fully... there's gonna be a lot of changes needed either way, but that'll be much less finicky I think.

I'll have a look at doing that shortly, might best be in a different PR. Will figure out if we keep this one or not in a bit.

EDIT: Josh raised some points offline that... complicate this consideration, so we'll chase this route a bit further and see where we get to...

vexx32 commented 1 year ago

I can put a version gate into the Get-ChocolateyPackage function, as we need it in the main module logic anyway for some of this... I think the bulk of the tests we remove --local-only from, and only keep using it in choco v1-specific tests.

vexx32 commented 1 year ago

@Windos I've split out additional tests to the choco v1 test file, and added some version gating where it looks appropriate. Additionally, tests not in the v1 test file have had their --local-only params removed wherever they query chocolatey directly.

I skimmed through the other modules' tests, but didn't see anything that looks like it needs worrying about with the v2 changes.

vexx32 commented 1 year ago

@Windos thanks for helping sort out the issues I was having here! 💜

I've rebased this against the latest master and tidied up the commit history, I think this should be good for review (assuming CI passes, which I think it should).