Skip to content

Commit

Permalink
Remove unnecessary os.Stat in eval cache (#624)
Browse files Browse the repository at this point in the history
* Remove unnecessary `os.Stat` in eval cache
Any time that stat returns a file stat, we're going to read the file anyways. Might as well jump straight to the read
`os.ReadFile` returns an error that matches `os.IsNotExist` just the same as `os.Stat`

* Add caching test
  • Loading branch information
julienduchesne authored Oct 14, 2021
1 parent ab2c791 commit 4271d75
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 6 deletions.
88 changes: 84 additions & 4 deletions pkg/jsonnet/eval_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,104 @@
package jsonnet

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const importTreeResult = `[
{
"breed": "apple",
"color": "red",
"creates": "o2",
"eats": "co2",
"keeps": "the world healthy",
"kind": "tree",
"needs": "water",
"size": "m"
},
{
"breed": "cherry",
"color": "red",
"creates": "o2",
"eats": "co2",
"keeps": "the world healthy",
"kind": "tree",
"needs": "water",
"size": "xs"
},
{
"breed": "peach",
"color": "orange",
"creates": "o2",
"eats": "co2",
"keeps": "the world healthy",
"kind": "tree",
"needs": "water",
"size": "s"
}
]
`

const thisFileResult = `{
"test": "testdata/thisFile/main.jsonnet"
}
`

// To be consistent with the jsonnet executable,
// when evaluating a file, `std.thisFile` should point to the given path
func TestEvaluateFile(t *testing.T) {
result, err := EvaluateFile("testdata/thisFile/main.jsonnet", Opts{})
assert.NoError(t, err)
assert.Equal(t, `{
"test": "testdata/thisFile/main.jsonnet"
}
`, result)
assert.Equal(t, thisFileResult, result)
}

func TestEvaluateFileDoesntExist(t *testing.T) {
result, err := EvaluateFile("testdata/doesnt-exist/main.jsonnet", Opts{})
assert.EqualError(t, err, "open testdata/doesnt-exist/main.jsonnet: no such file or directory")
assert.Equal(t, "", result)
}

func TestEvaluateFileWithCaching(t *testing.T) {
tmp, err := os.MkdirTemp("", "test-tanka-caching")
require.NoError(t, err)
defer os.RemoveAll(tmp)
cachePath := filepath.Join(tmp, "cache") // Should be created during caching

// Evaluate two files
result, err := EvaluateFile("testdata/thisFile/main.jsonnet", Opts{CachePath: cachePath})
assert.NoError(t, err)
assert.Equal(t, thisFileResult, result)
result, err = EvaluateFile("testdata/importTree/main.jsonnet", Opts{CachePath: cachePath})
assert.NoError(t, err)
assert.Equal(t, importTreeResult, result)

// Check that we have two entries in the cache
readCache, err := os.ReadDir(cachePath)
require.NoError(t, err)
assert.Len(t, readCache, 2)

// Evaluate two files again, same result
result, err = EvaluateFile("testdata/thisFile/main.jsonnet", Opts{CachePath: cachePath})
assert.NoError(t, err)
assert.Equal(t, thisFileResult, result)
result, err = EvaluateFile("testdata/importTree/main.jsonnet", Opts{CachePath: cachePath})
assert.NoError(t, err)
assert.Equal(t, importTreeResult, result)

// Modify the cache items
for _, entry := range readCache {
require.NoError(t, os.WriteFile(filepath.Join(cachePath, entry.Name()), []byte(entry.Name()), 0666))
}

// Evaluate two files again, modified cache is returned instead of the actual result
result, err = EvaluateFile("testdata/thisFile/main.jsonnet", Opts{CachePath: cachePath})
assert.NoError(t, err)
assert.Equal(t, "BYfdlr1ZOVwiOfbd89JYTcK-eRQh05bi8ky3k1vVW5o=.json", result)
result, err = EvaluateFile("testdata/importTree/main.jsonnet", Opts{CachePath: cachePath})
assert.NoError(t, err)
assert.Equal(t, "R_3hy-dRfOwXN-fezQ50ZF4dnrFcBcbQ9LztR_XWzJA=.json", result)
}
3 changes: 1 addition & 2 deletions pkg/jsonnet/evalcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ func (c *FileEvalCache) Get(hash string) (string, error) {
return "", err
}

if _, err := os.Stat(cachePath); err == nil {
bytes, err := os.ReadFile(cachePath)
if bytes, err := os.ReadFile(cachePath); err == nil {
return string(bytes), err
} else if !os.IsNotExist(err) {
return "", err
Expand Down

0 comments on commit 4271d75

Please sign in to comment.