Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jul 15, 2022

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?
Draft of a gRPC test-suite.
There are helper functions to:

  • create a test environment
  • download and extract artifacts (not currently used in the gRPC testing but I had these functions lingering in my local git so I decided to include them anyway)
  • run the arduino-cli (it is confined in the environment as we do in our integration tests)
  • run the arduino-cli as a daemon (the cli daemon is started in the background and commands may be sent via gRPC)
  • the output is partially colored/decorated

I've put everything inside the internal package, but I see that some of those objects may be part of a more general-purpose library (like the testsuite.Environment object may be potentially reused in other projects.

This implementation has some overlap with our current integration tests in python. As I see it, it has the potential to completely replace pytest if we properly expand it.

Does this PR introduce a breaking change, and is titled accordingly?
No

@cmaglie cmaglie self-assigned this Jul 15, 2022
@cmaglie
Copy link
Member Author

cmaglie commented Jul 15, 2022

https://github.com/arduino/arduino-cli/runs/7356179761?check_suite_focus=true#step:7:1328 this is an real bug that the testsuite is triggering

@cmaglie cmaglie force-pushed the grpc_testsuite branch 3 times, most recently from b650e9d to c6cfe21 Compare July 29, 2022 09:12
@per1234 per1234 added type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure topic: gRPC Related to the gRPC interface labels Jul 31, 2022
@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #1806 (757fa06) into master (2dd8976) will increase coverage by 0.77%.
The diff coverage is 79.19%.

@@            Coverage Diff             @@
##           master    #1806      +/-   ##
==========================================
+ Coverage   35.50%   36.27%   +0.77%     
==========================================
  Files         230      232       +2     
  Lines       19532    19497      -35     
==========================================
+ Hits         6934     7073     +139     
+ Misses      11773    11594     -179     
- Partials      825      830       +5     
Flag Coverage Δ
unit 36.27% <79.19%> (+0.77%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/integrationtest/arduino-cli.go 79.19% <79.19%> (ø)
arduino/cores/packagemanager/install_uninstall.go 9.96% <0.00%> (-5.12%) ⬇️
arduino/cores/packagemanager/download.go 22.89% <0.00%> (-2.11%) ⬇️
cli/update/update.go 0.00% <0.00%> (ø)
commands/core/install.go 0.00% <0.00%> (ø)
commands/core/upgrade.go 0.00% <0.00%> (ø)
commands/core/download.go 0.00% <0.00%> (ø)
commands/daemon/daemon.go 0.00% <0.00%> (ø)
commands/core/uninstall.go 0.00% <0.00%> (ø)
commands/bundled_tools.go
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@cmaglie cmaglie marked this pull request as ready for review August 2, 2022 09:16
@cmaglie cmaglie requested review from per1234 and umbynos August 2, 2022 09:16
@cmaglie
Copy link
Member Author

cmaglie commented Aug 2, 2022

This PR has currently evolved:

  • we internally decided to port the integration tests from python to golang and remove dependencies from python: this task will be done after this PR incrementally over time
  • some of the generic utility functions have been moved into a dedicated library in https://go.bug.st/testsuite

@cmaglie cmaglie force-pushed the grpc_testsuite branch 2 times, most recently from ac27b0c to 6601e97 Compare August 3, 2022 08:37
Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also would bettere IMHO to use a different name for this variable. And a more clear comment here

@cmaglie cmaglie requested a review from umbynos August 5, 2022 16:14
Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@silvanocerza
Copy link
Contributor

https://media.giphy.com/media/SCkJiHl67sDMQ/giphy.gif

Co-authored-by: per1234 <accounts@perglass.com>
Co-authored-by: per1234 <accounts@perglass.com>
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work Cristian. Thanks!

@cmaglie cmaglie merged commit 869c971 into arduino:master Aug 9, 2022
@cmaglie cmaglie deleted the grpc_testsuite branch August 9, 2022 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: gRPC Related to the gRPC interface topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants