From 3f8b1eb1a1446a203005ccff6661f8e735263c5e Mon Sep 17 00:00:00 2001
From: Luther Monson <luther.monson@gmail.com>
Date: Mon, 16 Jan 2023 01:22:59 -0700
Subject: [PATCH] adding more tests and light refactor

---
 cluster_firewall.go | 35 ++++++++++++++++-------------------
 tasks_test.go       | 19 +++++++++++++++++++
 virtual_machines.go |  9 +++++----
 3 files changed, 40 insertions(+), 23 deletions(-)
 create mode 100644 tasks_test.go

diff --git a/cluster_firewall.go b/cluster_firewall.go
index 8ea6b8d..5807217 100644
--- a/cluster_firewall.go
+++ b/cluster_firewall.go
@@ -25,29 +25,26 @@ func (cl *Cluster) FWGroup(name string) (group *FirewallSecurityGroup, err error
 	return
 }
 
-func (cl *Cluster) NewFWGroup(group *FirewallSecurityGroup) (err error) {
-	err = cl.client.Post(fmt.Sprintf("/cluster/firewall/groups"), group, &group)
-	return
+func (cl *Cluster) NewFWGroup(group *FirewallSecurityGroup) error {
+	return cl.client.Post(fmt.Sprintf("/cluster/firewall/groups"), group, &group)
 }
 
-func (g *FirewallSecurityGroup) GetRules() (rules []*FirewallRule, err error) {
-	err = g.client.Get(fmt.Sprintf("/cluster/firewall/groups/%s", g.Group), &g.Rules)
-	rules = g.Rules
-	return
+func (g *FirewallSecurityGroup) GetRules() ([]*FirewallRule, error) {
+	return g.Rules, g.client.Get(fmt.Sprintf("/cluster/firewall/groups/%s", g.Group), &g.Rules)
 }
-func (g *FirewallSecurityGroup) Delete() (err error) {
-	err = g.client.Delete(fmt.Sprintf("/cluster/firewall/groups/%s", g.Group), nil)
-	return
+
+func (g *FirewallSecurityGroup) Delete() error {
+	return g.client.Delete(fmt.Sprintf("/cluster/firewall/groups/%s", g.Group), nil)
 }
-func (g *FirewallSecurityGroup) RuleCreate(rule *FirewallRule) (err error) {
-	err = g.client.Post(fmt.Sprintf("/cluster/firewall/groups/%s", g.Group), rule, nil)
-	return
+
+func (g *FirewallSecurityGroup) RuleCreate(rule *FirewallRule) error {
+	return g.client.Post(fmt.Sprintf("/cluster/firewall/groups/%s", g.Group), rule, nil)
 }
-func (g *FirewallSecurityGroup) RuleUpdate(rule *FirewallRule) (err error) {
-	err = g.client.Put(fmt.Sprintf("/cluster/firewall/groups/%s/%d", g.Group, rule.Pos), rule, nil)
-	return
+
+func (g *FirewallSecurityGroup) RuleUpdate(rule *FirewallRule) error {
+	return g.client.Put(fmt.Sprintf("/cluster/firewall/groups/%s/%d", g.Group, rule.Pos), rule, nil)
 }
-func (g *FirewallSecurityGroup) RuleDelete(rulePos int) (err error) {
-	err = g.client.Delete(fmt.Sprintf("/cluster/firewall/groups/%s/%d", g.Group, rulePos), nil)
-	return
+
+func (g *FirewallSecurityGroup) RuleDelete(rulePos int) error {
+	return g.client.Delete(fmt.Sprintf("/cluster/firewall/groups/%s/%d", g.Group, rulePos), nil)
 }
diff --git a/tasks_test.go b/tasks_test.go
new file mode 100644
index 0000000..91d721f
--- /dev/null
+++ b/tasks_test.go
@@ -0,0 +1,19 @@
+package proxmox
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewTask(t *testing.T) {
+	upid := NewTask(UPID(""), &Client{})
+	assert.Nil(t, upid)
+
+	task := NewTask(UPID("UPID:nodename:00388B23:02D69651:63C4F6AF:tasktype:100:root@pam:"), &Client{})
+	assert.Equal(t, "nodename", task.Node)
+	assert.Equal(t, "100", task.ID)
+	assert.Equal(t, "tasktype", task.Type)
+	assert.Equal(t, "root@pam", task.User)
+	assert.Equal(t, UPID("UPID:nodename:00388B23:02D69651:63C4F6AF:tasktype:100:root@pam:"), task.UPID)
+}
diff --git a/virtual_machines.go b/virtual_machines.go
index cd3e952..73f9489 100644
--- a/virtual_machines.go
+++ b/virtual_machines.go
@@ -275,13 +275,13 @@ func (v *VirtualMachine) AgentSetUserPassword(password string, username string)
 	err = v.client.Post(fmt.Sprintf("/nodes/%s/qemu/%d/agent/set-user-password", node.Name, v.VMID), map[string]string{"password": password, "username": username}, nil)
 
 	return
-
 }
 
 func (v *VirtualMachine) FirewallOptionGet() (firewallOption *FirewallVirtualMachineOption, err error) {
 	err = v.client.Get(fmt.Sprintf("/nodes/%s/qemu/%d/firewall/options", v.Node, v.VMID), firewallOption)
 	return
 }
+
 func (v *VirtualMachine) FirewallOptionSet(firewallOption *FirewallVirtualMachineOption) (err error) {
 	err = v.client.Put(fmt.Sprintf("/nodes/%s/qemu/%d/firewall/options", v.Node, v.VMID), firewallOption, nil)
 	return
@@ -296,13 +296,14 @@ func (v *VirtualMachine) FirewallRulesCreate(rule *FirewallRule) (err error) {
 	err = v.client.Post(fmt.Sprintf("/nodes/%s/qemu/%d/firewall/rules", v.Node, v.VMID), rule, nil)
 	return
 }
+
 func (v *VirtualMachine) FirewallRulesUpdate(rule *FirewallRule) (err error) {
 	err = v.client.Put(fmt.Sprintf("/nodes/%s/qemu/%d/firewall/rules/%d", v.Node, v.VMID, rule.Pos), rule, nil)
 	return
 }
-func (v *VirtualMachine) FirewallRulesDelete(rulePos int) (err error) {
-	err = v.client.Delete(fmt.Sprintf("/nodes/%s/qemu/%d/firewall/rules/%d", v.Node, v.VMID, rulePos), nil)
-	return
+
+func (v *VirtualMachine) FirewallRulesDelete(rulePos int) error {
+	return v.client.Delete(fmt.Sprintf("/nodes/%s/qemu/%d/firewall/rules/%d", v.Node, v.VMID, rulePos), nil)
 }
 
 func (v *VirtualMachine) NewSnapshot(name string) (task *Task, err error) {