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.

Sign up for GitHub

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

Allow importing helper/utility files in test files #1030

Merged
merged 3 commits into from
May 23, 2023
Merged

Allow importing helper/utility files in test files #1030

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44d85ad
Allow importing helper/utility files in test files
m-Peter May 18, 2023
6c6d174
Use map literal for initializing testFiles
m-Peter May 18, 2023
e920d18
Move "_helper" magic string to constant
m-Peter May 18, 2023
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
22 changes: 22 additions & 0 deletions flowkit/tests/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,15 @@ var ScriptImport = Resource{
`),
}

var HelperImport = Resource{
Filename: "test_helpers.cdc",
Source: []byte(`
pub fun double(_ x: Int): Int {
return x * 2
}
`),
}

var TestScriptSimple = Resource{
Filename: "./testScriptSimple.cdc",
Source: []byte(`
Expand Down Expand Up @@ -354,6 +363,18 @@ var TestScriptWithImport = Resource{
`),
}

var TestScriptWithHelperImport = Resource{
Filename: "testScriptWithHelperImport.cdc",
Source: []byte(`
import Test
import "test_helpers.cdc"

pub fun testDouble() {
Test.expect(double(2), Test.equal(4))
}
`),
}

var TestScriptWithRelativeImports = Resource{
Filename: "testScriptWithRelativeImport.cdc",
Source: []byte(`
Expand Down Expand Up @@ -437,6 +458,7 @@ var resources = []Resource{
ContractSimpleUpdated,
TransactionSimple,
ScriptImport,
HelperImport,
ContractA,
ContractB,
ContractC,
Expand Down
11 changes: 10 additions & 1 deletion internal/test/test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,17 @@ func importResolver(scriptPath string, state *flowkit.State) cdcTests.ImportReso
if !isFileImport {
return "", fmt.Errorf("cannot import from %s", location)
}

relativePath := stringLocation.String()

if strings.Contains(relativePath, "_helper") {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe strip the file extension (path/filepath Ext()) and check the plain filename contains this as a suffix. Or is the idea that this allows both _helper and e.g. _helpers like in the two test files?

Also, maybe refactor this magic string into a constant and add a comment explaining it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or is the idea that this allows both _helper and e.g. _helpers like in the two test files?

Yes, exactly 👍 It might be the case that developers will put everything in a single helper file, or split them in several files. So I wanted to allow some flexibility in both plural & singular names.

Also, maybe refactor this magic string into a constant and add a comment explaining it.

Fixed in e920d18

importedScriptFilePath := absolutePath(scriptPath, relativePath)
scriptCode, err := state.ReadFile(importedScriptFilePath)
if err != nil {
return "", nil
}
return string(scriptCode), nil
}

contractFound := false
for _, contract := range *state.Contracts() {
if strings.Contains(relativePath, contract.Location) {
Expand Down
15 changes: 15 additions & 0 deletions internal/test/test_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ func TestExecutingTests(t *testing.T) {
assert.NoError(t, results[script.Filename][0].Error)
})

t.Run("with helper script import", func(t *testing.T) {
t.Parallel()
_, state, _ := util.TestMocks(t)

// Execute script
script := tests.TestScriptWithHelperImport
testFiles := make(map[string][]byte, 0)
testFiles[script.Filename] = script.Source
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
testFiles := make(map[string][]byte, 0)
testFiles[script.Filename] = script.Source
testFiles := map[string][]byte{
script.Filename: script.Source,
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 6c6d174

results, _, err := testCode(testFiles, state, false)

require.NoError(t, err)
require.Len(t, results, 1)
assert.NoError(t, results[script.Filename][0].Error)
})

t.Run("with missing contract location from config", func(t *testing.T) {
t.Parallel()

Expand Down