Skip to content
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

Update test command with latest features from Cadence testing framework #1227

Merged
merged 16 commits into from
Oct 25, 2023

Conversation

m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Oct 5, 2023

Depends on: onflow/cadence-tools#210
Work towards: onflow/developer-grants#216

Description

Incorporates the latest features from the Cadence testing framework:

  • Deployment of contracts is defined in a new testing network
  • Adds flag for random test case execution
  • Updates importResolver to handle AddressLocation

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@m-Peter m-Peter force-pushed the update-tests-with-latest-features branch from e700b17 to 1706dfa Compare October 19, 2023 14:54
@m-Peter m-Peter marked this pull request as ready for review October 20, 2023 06:56
@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (c2d0040) 38.65% compared to head (7aef0fe) 40.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1227      +/-   ##
==========================================
+ Coverage   38.65%   40.67%   +2.02%     
==========================================
  Files          38       38              
  Lines        1948     1974      +26     
==========================================
+ Hits          753      803      +50     
+ Misses       1107     1082      -25     
- Partials       88       89       +1     
Flag Coverage Δ
unittests 40.67% <79.66%> (+2.02%) ⬆️

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

Files Coverage Δ
internal/test/test.go 67.62% <79.66%> (+28.68%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

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

Nice!

flowkit/go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@m-Peter m-Peter requested a review from SupunS October 23, 2023 09:31
internal/test/test.go Outdated Show resolved Hide resolved
internal/test/test.go Outdated Show resolved Hide resolved
internal/test/test.go Outdated Show resolved Hide resolved
internal/test/test.go Outdated Show resolved Hide resolved
internal/test/test.go Outdated Show resolved Hide resolved
@m-Peter m-Peter requested a review from SupunS October 24, 2023 09:25
@bjartek
Copy link
Collaborator

bjartek commented Oct 24, 2023

LGTM nice changes

@m-Peter m-Peter mentioned this pull request Oct 24, 2023
Copy link
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

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

Nice work! 👏

internal/test/test.go Outdated Show resolved Hide resolved
internal/test/test.go Outdated Show resolved Hide resolved
@@ -35,6 +35,7 @@ require (
cloud.google.com/go/compute/metadata v0.2.3 // indirect
cloud.google.com/go/iam v1.1.1 // indirect
cloud.google.com/go/kms v1.12.1 // indirect
github.com/DataDog/zstd v1.4.5 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Why is datadog added here? Is this adding some sort of reporting?

Copy link
Collaborator Author

@m-Peter m-Peter Oct 25, 2023

Choose a reason for hiding this comment

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

It is an indirect dependency from some direct dependency:

github.com/DataDog/zstd
This module is necessary because [github.com/DataDog/zstd](vscode-file://vscode-app/snap/code/143/usr/share/code/resources/app/out/vs/code/electron-sandbox/workbench/workbench.html) is imported in:

- github.com/onflow/flow-cli/internal/accounts
-- github.com/onflow/flow-cli/flowkit/gateway
--- github.com/onflow/flow-go/model/flow
---- github.com/onflow/flow-go/model/flow.test
----- github.com/onflow/flow-go/utils/unittest
------ github.com/cockroachdb/pebble
------- github.com/cockroachdb/pebble/sstable

@@ -49,6 +50,12 @@ require (
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
github.com/cloudflare/circl v1.1.0 // indirect
github.com/cockroachdb/errors v1.8.1 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Is cockroachdb needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here:

github.com/cockroachdb/errors
This module is necessary because [github.com/cockroachdb/errors](vscode-file://vscode-app/snap/code/143/usr/share/code/resources/app/out/vs/code/electron-sandbox/workbench/workbench.html) is imported in:

- github.com/onflow/flow-cli/internal/accounts
-- github.com/onflow/flow-cli/flowkit/gateway
--- github.com/onflow/flow-go/model/flow
---- github.com/onflow/flow-go/model/flow.test
----- github.com/onflow/flow-go/utils/unittest
------ github.com/cockroachdb/pebble

@chasefleming chasefleming merged commit c615d33 into onflow:master Oct 25, 2023
7 checks passed
@chasefleming chasefleming added the Improvement Technical work without new features, refactoring, improving tests label Oct 30, 2023
@m-Peter m-Peter deleted the update-tests-with-latest-features branch November 15, 2023 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Technical work without new features, refactoring, improving tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants