search

hashicorp / packer-plugin-vsphere

Packer plugin for VMware vSphere Builder
https://www.packer.io/docs/builders/vsphere
Mozilla Public License 2.0
96 stars 93 forks source link

fix: use cdroms len instead of ISOPaths len #394

Closed Hi-Angel closed 6 months ago

Hi-Angel commented 6 months ago
  1. st commit improves code coverage by factoring out a function to list cdrom devices. Although, there's very little change in the test from that, bug I guess it's good to have a better testing.
  2. nd commit is the fix. As described in the commit, previously we used ISOPaths to figure out amount of cdroms, but that's not really correct and this assumption breaks when remove_cdrom is set. Fix the problem by making use of the function listing the devices that was added in prev. commit.
  3. rd commit is just a fix to the tests that nils out the list of ISOPaths added in EjectCdrom, which seems to be correct because after it's called we should no longer have paths mounted, only the actual cdrom hw devices. With that said, it seems this does not have any influence on the tests right now.

Fixes: https://github.com/hashicorp/packer-plugin-vsphere/issues/393

Hi-Angel commented 6 months ago

Okay, ready for review.

  1. st commit improves code coverage by factoring out a function to list cdrom devices. Although, there's very little change in the test from that, bug I guess it's good to have a better testing.
  2. nd commit is the fix. As described in the commit, previously we used ISOPaths to figure out amount of cdroms, but that's not really correct and this assumption breaks when remove_cdrom is set. Fix the problem by making use of the function listing the devices that was added in prev. commit.
  3. rd commit is just a fix to the tests that nils out the list of ISOPaths added in EjectCdrom, which seems to be correct because after it's called we should no longer have paths mounted, only the actual cdrom hw devices. With that said, it seems this does not have any influence on the tests right now.

Please review 😊

Hi-Angel commented 6 months ago

Cool, thank you!