Use Process.Kill instead which is (hopefully) cross-platform enough. Improve test reliability.

This commit is contained in:
Justin Hawkins 2022-01-06 16:19:22 +10:30
parent 21f9e71d6d
commit f487ff0371
3 changed files with 23 additions and 14 deletions

View File

@ -5,12 +5,12 @@ import (
"io"
"log"
"net/url"
"os"
"os/exec"
"regexp"
"strconv"
"strings"
"sync"
"syscall"
"time"
"github.com/tardisx/gropple/config"
@ -20,7 +20,7 @@ type Download struct {
Id int `json:"id"`
Url string `json:"url"`
PopupUrl string `json:"popup_url"`
Pid int `json:"pid"`
Process *os.Process `json:"-"`
ExitCode int `json:"exit_code"`
State string `json:"state"`
DownloadProfile config.DownloadProfile `json:"download_profile"`
@ -59,7 +59,7 @@ func (dls Downloads) StartQueued(maxRunning int) {
if dl.State == "queued" && (maxRunning == 0 || active[dl.domain()] < maxRunning) {
dl.State = "downloading"
active[dl.domain()]++
log.Printf("Starting download for %#v", dl)
log.Printf("Starting download for id:%d (%s)", dl.Id, dl.Url)
dl.mutex.Unlock()
go func() { dl.Begin() }()
} else {
@ -101,9 +101,9 @@ func (dl *Download) Queue() {
func (dl *Download) Stop() {
log.Printf("stopping the download")
dl.mutex.Lock()
dl.Log = append(dl.Log, "aborted by user")
defer dl.mutex.Unlock()
syscall.Kill(dl.Pid, syscall.SIGTERM)
dl.Process.Kill()
}
func (dl *Download) domain() string {
@ -129,8 +129,8 @@ func (dl *Download) Begin() {
cmdSlice := []string{}
cmdSlice = append(cmdSlice, dl.DownloadProfile.Args...)
// only add the url if it's not empty. This helps us with testing
if dl.Url != "" {
// only add the url if it's not empty or an example URL. This helps us with testing
if !(dl.Url == "" || strings.Contains(dl.domain(), "example.org")) {
cmdSlice = append(cmdSlice, dl.Url)
}
@ -155,7 +155,7 @@ func (dl *Download) Begin() {
return
}
log.Printf("Starting %v", cmd)
log.Printf("Executing command: %v", cmd)
err = cmd.Start()
if err != nil {
dl.State = "failed"
@ -164,7 +164,7 @@ func (dl *Download) Begin() {
dl.Log = append(dl.Log, fmt.Sprintf("error starting command '%s': %v", dl.DownloadProfile.Command, err))
return
}
dl.Pid = cmd.Process.Pid
dl.Process = cmd.Process
var wg sync.WaitGroup
@ -185,6 +185,7 @@ func (dl *Download) Begin() {
cmd.Wait()
dl.mutex.Lock()
log.Printf("Process finished for id: %d (%v)", dl.Id, cmd)
dl.State = "complete"
dl.Finished = true

View File

@ -2,6 +2,7 @@ package download
import (
"testing"
"time"
"github.com/tardisx/gropple/config"
)
@ -68,13 +69,14 @@ func TestUpdateMetadata(t *testing.T) {
func TestQueue(t *testing.T) {
conf := config.TestConfig()
new1 := Download{Id: 1, Url: "http://domain1.com/foo", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new2 := Download{Id: 2, Url: "http://domain1.com/foo", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new3 := Download{Id: 3, Url: "http://domain1.com/foo", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new4 := Download{Id: 4, Url: "http://company.org/", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new1 := Download{Id: 1, Url: "http://sub.example.org/foo1", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new2 := Download{Id: 2, Url: "http://sub.example.org/foo2", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new3 := Download{Id: 3, Url: "http://sub.example.org/foo3", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
new4 := Download{Id: 4, Url: "http://example.org/", State: "queued", DownloadProfile: conf.DownloadProfiles[0], Config: conf}
dls := Downloads{&new1, &new2, &new3, &new4}
dls.StartQueued(1)
time.Sleep(time.Millisecond * 100)
if dls[0].State == "queued" {
t.Error("#1 was not started")
}
@ -87,16 +89,20 @@ func TestQueue(t *testing.T) {
// this should start no more, as one is still going
dls.StartQueued(1)
time.Sleep(time.Millisecond * 100)
if dls[1].State != "queued" {
t.Error("#2 was started when it should not be")
}
dls.StartQueued(2)
time.Sleep(time.Millisecond * 100)
if dls[1].State == "queued" {
t.Error("#2 was not started but it should be")
}
dls.StartQueued(2)
time.Sleep(time.Millisecond * 100)
if dls[3].State == "queued" {
t.Error("#4 was not started but it should be")
}
@ -108,6 +114,7 @@ func TestQueue(t *testing.T) {
dls[3].State = "queued"
dls.StartQueued(0)
time.Sleep(time.Millisecond * 100)
// they should all be going
if dls[0].State == "queued" || dls[1].State == "queued" || dls[2].State == "queued" || dls[3].State == "queued" {
@ -121,10 +128,12 @@ func TestQueue(t *testing.T) {
dls[3].State = "queued"
dls.StartQueued(2)
time.Sleep(time.Millisecond * 100)
// first two should be running, third not (same domain) and 4th running (different domain)
if dls[0].State == "queued" || dls[1].State == "queued" || dls[2].State != "queued" || dls[3].State == "queued" {
t.Error("incorrect queued")
}
}

View File

@ -258,7 +258,6 @@ func fetchInfoOneRESTHandler(w http.ResponseWriter, r *http.Request) {
}
if thisReq.Action == "stop" {
thisDownload.Stop()
succRes := successResponse{Success: true, Message: "download stopped"}
succResB, _ := json.Marshal(succRes)