8000 [breaking] Refactor `DownloadProgress` gRPC message: more clear field meaning. by cmaglie · Pull Request #1875 · arduino/arduino-cli · GitHub
[go: up one dir, main page]

Skip to content

[breaking] Refactor DownloadProgress gRPC message: more clear field meaning. #1875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updated integration tests
  • Loading branch information
cmaglie committed Sep 22, 2022
commit 350e90f34c33620c80440c1e07e22137d91a8ff9
65 changes: 22 additions & 43 deletions internal/integrationtest/daemon/daemon_core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,59 +41,38 @@ func TestDaemonCoreUpdateIndex(t *testing.T) {
` "http://downloads.arduino.cc/package_inexistent_index.json"]`)
require.NoError(t, err)

analyzeUpdateIndexClient := func(cl commands.ArduinoCoreService_UpdateIndexClient) (error, map[string]*commands.DownloadProgressEnd) {
analyzer := NewDownloadProgressAnalyzer(t)
for {
msg, err := cl.Recv()
// fmt.Println("DOWNLOAD>", msg)
if err == io.EOF {
return nil, analyzer.Results
}
if err != nil {
return err, analyzer.Results
}
require.NoError(t, err)
analyzer.Process(msg.GetDownloadProgress())
}
}

{
cl, err := grpcInst.UpdateIndex(context.Background(), true)
require.NoError(t, err)
res, err := analyzeUpdateIndexStream(t, cl)
err, res := analyzeUpdateIndexClient(cl)
require.NoError(t, err)
require.Len(t, res, 1)
require.True(t, res["https://downloads.arduino.cc/packages/package_index.tar.bz2"].Successful)
require.True(t, res["https://downloads.arduino.cc/packages/package_index.tar.bz2"].Success)
}
{
cl, err := grpcInst.UpdateIndex(context.Background(), false)
require.NoError(t, err)
res, err := analyzeUpdateIndexStream(t, cl)
err, res := analyzeUpdateIndexClient(cl)
require.Error(t, err)
require.Len(t, res, 3)
require.True(t, res["https://downloads.arduino.cc/packages/package_index.tar.bz2"].Successful)
require.True(t, res["http://arduino.esp8266.com/stable/package_esp8266com_index.json"].Successful)
require.False(t, res["http://downloads.arduino.cc/package_inexistent_index.json"].Successful)
}
}

// analyzeUpdateIndexStream runs an update index checking if the sequence of DownloadProgress and
// DownloadResult messages is correct. It returns a map reporting all the DownloadResults messages
// received (it maps urls to DownloadResults).
func analyzeUpdateIndexStream(t *testing.T, cl commands.ArduinoCoreService_UpdateIndexClient) (map[string]*commands.DownloadResult, error) {
ongoingDownload := ""
results := map[string]*commands.DownloadResult{}
for {
msg, err := cl.Recv()
if err == io.EOF {
return results, nil
}
if err != nil {
return results, err
}
require.NoError(t, err)
fmt.Printf("UPDATE> %+v\n", msg)
if progress := msg.GetDownloadProgress(); progress != nil {
if progress.Url != "" && progress.Url != ongoingDownload {
require.Empty(t, ongoingDownload, "DownloadProgress: started a download without 'completing' the previous one")
ongoingDownload = progress.Url
}
if progress.Completed {
require.NotEmpty(t, ongoingDownload, "DownloadProgress: received a 'completed' notification but never initiated a download")
ongoingDownload = ""
}
if progress.Downloaded > 0 {
require.NotEmpty(t, ongoingDownload, "DownloadProgress: received a download update but never initiated a download")
}
} else if result := msg.GetDownloadResult(); result != nil {
require.Empty(t, ongoingDownload, "DownloadResult: got a download result without completing the current download first")
results[result.Url] = result
} else {
require.FailNow(t, "DownloadProgress: received an empty message (without a Progress or a Result)")
}
require.True(t, res["https://downloads.arduino.cc/packages/package_index.tar.bz2"].Success)
require.True(t, res["http://arduino.esp8266.com/stable/package_esp8266com_index.json"].Success)
require.False(t, res["http://downloads.arduino.cc/package_inexistent_index.json"].Success)
}
}
58 changes: 58 additions & 0 deletions internal/integrationtest/daemon/download_progress_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// This file is part of arduino-cli.
//
// Copyright 2022 ARDUINO SA (http://www.arduino.cc/)
//
// This software is released under the GNU General Public License version 3,
// which covers the main part of arduino-cli.
// The terms of this license can be found at:
// https://www.gnu.org/licenses/gpl-3.0.en.html
//
// You can be released from the requirements of the above licenses by purchasing
// a commercial license. Buying such a license is mandatory if you want to
// modify or otherwise use the software for commercial activities involving the
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to license@arduino.cc.

package daemon_test

import (
"testing"

"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/stretchr/testify/require"
)

// DownloadProgressAnalyzer analyzes DownloadProgress messages for consistency
type DownloadProgressAnalyzer struct {
t *testing.T
ongoingDownload string
Results map[string]*commands.DownloadProgressEnd
}

// NewDownloadProgressAnalyzer creates a new DownloadProgressAnalyzer
func NewDownloadProgressAnalyzer(t *testing.T) *DownloadProgressAnalyzer {
return &DownloadProgressAnalyzer{
t: t,
Results: map[string]*commands.DownloadProgressEnd{},
}
}

// Process the given DownloadProgress message. If inconsistencies are detected the
// test will fail.
func (a *DownloadProgressAnalyzer) Process(progress *commands.DownloadProgress) {
if progress == nil {
return
}
if start := progress.GetStart(); start != nil {
require.Empty(a.t, a.ongoingDownload, "DownloadProgressStart: started a download without 'completing' the previous one")
a.ongoingDownload = start.Url
} else if update := progress.GetUpdate(); update != nil {
require.NotEmpty(a.t, a.ongoingDownload, "DownloadProgressUpdate: received update, but the download is not yet started...")
} else if end := progress.GetEnd(); end != nil {
require.NotEmpty(a.t, a.ongoingDownload, "DownloadProgress: received a 'completed' notification but never initiated a download")
a.Results[a.ongoingDownload] = end
a.ongoingDownload = ""
} else {
require.FailNow(a.t, "DownloadProgress: received an empty DownloadProgress (without Start, Update or End)")
}
}
13 changes: 6 additions & 7 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ def test_core_updateindex_url_not_found(run_command, httpserver):

result = run_command(["core", "update-index", f"--additional-urls={url}"])
assert result.failed
lines = [l.strip() for l in result.stderr.splitlines()]
assert f"Error updating index: Error downloading index '{url}': Server responded with: 404 NOT FOUND" in lines
linesout = [l.strip() for l in result.stdout.splitlines()]
assert "Downloading index: test_index.json Server responded with: 404 NOT FOUND" in linesout
lineserr = [l.strip() for l in result.stderr.splitlines()]
assert "Some indexes could not be updated." in lineserr


def test_core_updateindex_internal_server_error(run_command, httpserver):
Expand All @@ -107,11 +109,8 @@ def test_core_updateindex_internal_server_error(run_command, httpserver):

result = run_command(["core", "update-index", f"--additional-urls={url}"])
assert result.failed
lines = [l.strip() for l in result.stderr.splitlines()]
assert (
f"Error updating index: Error downloading index '{url}': Server responded with: 500 INTERNAL SERVER ERROR"
in lines
)
lines = [l.strip() for l in result.stdout.splitlines()]
assert "Downloading index: test_index.json Server responded with: 500 INTERNAL SERVER ERROR" in lines


def test_core_install_without_updateindex(run_command):
Expand Down
11 changes: 4 additions & 7 deletions test/test_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ def test_update_with_url_not_found(run_command, httpserver):

res = run_command(["update", f"--additional-urls={url}"])
assert res.failed
lines = [l.strip() for l in res.stderr.splitlines()]
assert f"Error updating index: Error downloading index '{url}': Server responded with: 404 NOT FOUND" in lines
lines = [l.strip() for l in res.stdout.splitlines()]
assert "Downloading index: test_index.json Server responded with: 404 NOT FOUND" in lines


def test_update_with_url_internal_server_error(run_command, httpserver):
Expand All @@ -73,11 +73,8 @@ def test_update_with_url_internal_server_error(run_command, httpserver):

res = run_command(["update", f"--additional-urls={url}"])
assert res.failed
lines = [l.strip() for l in res.stderr.splitlines()]
assert (
f"Error updating index: Error downloading index '{url}': Server responded with: 500 INTERNAL SERVER ERROR"
in lines
)
lines = [l.strip() for l in res.stdout.splitlines()]
assert "Downloading index: test_index.json Server responded with: 500 INTERNAL SERVER ERROR" in lines


def test_update_showing_outdated_using_library_with_invalid_version(run_command, data_dir):
Expand Down
0