Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
16 changes: 16 additions & 0 deletions features/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"reflect"
"testing"

"github.com/EarthBuild/earthbuild/internal/earthfile"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -36,6 +37,21 @@ func TestMustParseVersion(t *testing.T) {
}
}

func TestGetRejectsFeatureFlagOverride(t *testing.T) {
t.Parallel()

// A bool feature flag given an explicit argument must be rejected. Mirrors
// the invalid-feature-flag-override.earth integration fixture end-to-end:
// the parser accepts the VERSION line (0.6 is a valid version), and Get
// rejects the malformed flag when it parses the VERSION arguments.
tree, err := earthfile.Parse("Earthfile", "VERSION --referenced-save-only=false 0.6")
require.NoError(t, err)

_, _, err = Get(tree.Version)
require.Error(t, err)
require.ErrorContains(t, err, "bool flag `--referenced-save-only' cannot have an argument")
}

func TestFeaturesStringEnabled(t *testing.T) {
t.Parallel()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
VERSION 0.8 --try # we should consider making this format valid, but for now it isn't and we should test it
5 changes: 5 additions & 0 deletions internal/earthfile/testdata/version/single-line.earth
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
VERSION 0.8

FROM alpine:3.24.0
test:
RUN echo "pass"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
VERSION 0.8
IMPORT ./subdir AS empty-earthfile-only-containing-a-version

test:
FROM alpine:3.24.0
RUN echo "pass"
1 change: 1 addition & 0 deletions internal/earthfile/testdata/version/version-only.earth
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
VERSION 0.8
61 changes: 61 additions & 0 deletions internal/earthfile/version_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package earthfile

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -25,3 +26,63 @@ func TestParseVersionFile_Error(t *testing.T) {
r.Error(err)
r.ErrorContains(err, "earthfile: unable to open file")
}

func TestVersionFixtures(t *testing.T) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These tests duplicate what the table tests in the TestParse already do.

Extend the table tests for missing scenarios in TestParse?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I retained the previous ast tests in internal/earthfile/testdata to avoid making big changes and keep the change scope as small as possible when implementing parsing.

In reality, the tests in this dir have been transferred into table tests in TestParse(). Effectively, these tests are run twice: as unit tests and as higher-level tests +test-ast later.

This requires a better understanding of what earthly debug ast does. My current assumption is we can remove "+test-ast" and internal/earthfile/testdata, and leave only unit tests.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Dug into what earthly debug ast actually does: it's a thin wrapper - earthfile.ParseFile + json.Marshal + print (cmd/earthly/subcmd/debug_cmds.go). No buildkit involvement beyond the test harness itself.

So +test-ast guards two things the unit tests don't:

  1. The JSON wire format. TestParse compares Go structs, so a struct-tag change would pass unit tests while silently changing debug ast output for any external consumer of that JSON. Worth checking whether anything (editor/LSP tooling?) still consumes it before declaring the format free to change.
  2. A realistic corpus - it parses the real integration Earthfiles under tests/. Downside: brittle goldens - two .ast.json files changed in this PR just because a dind image bump touched the source .earth files.

Proposal: replace rather than delete. A plain Go golden test in internal/earthfile that walks the existing *.ast.json files, parses the paired .earth, marshals, and diffs normalised JSON keeps both of the above at unit-test cost. Then +test-ast, tests+ast-test-input, and the binary round-trip can go (perhaps keeping one debug ast CLI smoke, same pattern as the slimmed tests/version).

On the duplication in this file: agreed - folded. The invalid VERSION cases now live in TestParseErrors, the valid spellings in a TestVersionVariants table with inline inputs asserting Version.Args, and internal/earthfile/testdata/ is deleted. (invalid-feature-flag-override.earth was already covered inline in features/features_test.go, so that fixture went too.) Will be in the next push.

t.Parallel()

validFixtures := []string{
"single-line.earth",
"single-line-with-args.earth",
"single-line-with-comment.earth",
"multi-line.earth",
"multi-line-with-comment.earth",
"multi-line-with-comment2.earth",
"multi-line-with-comment3.earth",
"multi-line-with-comment4.earth",
"multi-line-with-args.earth",
"multi-line-with-args2.earth",
"multi-line-with-empty-newline.earth",
"version-only-import.earth",
"version-only.earth",
"comment-and-whitespace-before-version.earth",
"whitespace-then-version.earth",
}

for _, fixture := range validFixtures {
t.Run(fixture, func(t *testing.T) {
t.Parallel()

tree, err := ParseFile(filepath.Join("testdata", "version", fixture))
require.NoError(t, err)
require.NotNil(t, tree.Version)
})
}
}

func TestInvalidVersionFixtures(t *testing.T) {
t.Parallel()

// The parser's version validator funnels every unsupported VERSION value
// (bad major/minor/patch, or an unrecognised trailing token) to the same
// message. Feature-flag validation (e.g. invalid-feature-flag-override.earth)
// lives in the features package, not here, so it is exercised by the
// integration tests under tests/version, not by this unit test.
const wantErr = "invalid VERSION in Earthfile, supported versions are 0.6, 0.7, or 0.8"

fixtures := []string{
"invalid-major-version.earth",
"invalid-minor-version.earth",
"invalid-patch-version.earth",
"invalid-format-version.earth",
}

for _, fixture := range fixtures {
t.Run(fixture, func(t *testing.T) {
t.Parallel()

_, err := ParseFile(filepath.Join("testdata", "version", fixture))
require.Error(t, err)
require.ErrorContains(t, err, wantErr)
})
}
}
78 changes: 5 additions & 73 deletions tests/version/Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,89 +5,21 @@ IMPORT .. AS tests

WORKDIR /test

test-single-line:
test-cli-smoke:
# Detailed VERSION syntax coverage lives in internal/earthfile/version_test.go. Keep one
# successful build, one version-only import, and one invalid-version CLI
# smoke here so the full command path still gets exercised.
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=single-line.earth --target=+test

test-single-line-with-args:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=single-line-with-args.earth --target=+test

test-single-line-with-comment:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=single-line-with-comment.earth --target=+test

test-multi-line:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line.earth --target=+test

test-multi-line-with-comment:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-comment.earth --target=+test

test-multi-line-with-comment2:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-comment2.earth --target=+test

test-multi-line-with-comment3:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-comment3.earth --target=+test

test-multi-line-with-comment4:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-comment4.earth --target=+test

test-multi-line-with-args:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-args.earth --target=+test

test-multi-line-with-args2:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-args2.earth --target=+test

test-multi-line-with-newline:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=multi-line-with-empty-newline.earth --target=+test

test-no-feature-flag-overrides:
DO --pass-args +RUN_EARTHLY_ARGS --should_fail=true --earthfile=invalid-feature-flag-override.earth --target=+test --output_contains="bool flag .--referenced-save-only. cannot have an argument"

test-version-only-import:
RUN mkdir subdir
RUN echo "VERSION 0.8" > subdir/Earthfile
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=version-only-import.earth --target=+test

# test-version-only-without-newline tests that earthly will still work with non-POSIX text files
test-version-only-without-newline:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=version-only.earth --target=+base
RUN test "$(cat Earthfile | wc -l)" = "0" # check Earthfile doesn't contain a newline

test-comment-and-whitespace-before-version:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=comment-and-whitespace-before-version.earth --target=+test

test-whitespace-then-version:
DO --pass-args +RUN_EARTHLY_ARGS --earthfile=whitespace-then-version.earth --target=+test

test-invalid-versions:
DO --pass-args +RUN_EARTHLY_ARGS --should_fail=true --earthfile=invalid-major-version.earth --target=+base
RUN acbgrep 'invalid VERSION in Earthfile, supported versions are 0.6, 0.7, or 0.8' earthly.output

DO --pass-args +RUN_EARTHLY_ARGS --should_fail=true --earthfile=invalid-minor-version.earth --target=+base
RUN acbgrep 'invalid VERSION in Earthfile, supported versions are 0.6, 0.7, or 0.8' earthly.output

DO --pass-args +RUN_EARTHLY_ARGS --should_fail=true --earthfile=invalid-patch-version.earth --target=+base
RUN acbgrep 'unexpected VERSION arguments; should be VERSION \[flags\] <major-version>.<minor-version>' earthly.output

DO --pass-args +RUN_EARTHLY_ARGS --should_fail=true --earthfile=invalid-format-version.earth --target=+base
RUN acbgrep 'unexpected VERSION arguments; should be VERSION \[flags\] <major-version>.<minor-version>' earthly.output

test-all:
BUILD +test-single-line
BUILD +test-single-line-with-args
BUILD +test-single-line-with-comment
BUILD +test-multi-line
BUILD +test-multi-line-with-comment
BUILD +test-multi-line-with-comment2
BUILD +test-multi-line-with-comment3
BUILD +test-multi-line-with-comment4
BUILD +test-multi-line-with-args
BUILD +test-multi-line-with-args2
BUILD +test-multi-line-with-newline
BUILD +test-version-only-without-newline
BUILD +test-comment-and-whitespace-before-version
BUILD +test-whitespace-then-version
BUILD +test-version-only-import
BUILD +test-invalid-versions
BUILD +test-no-feature-flag-overrides
BUILD +test-cli-smoke

RUN_EARTHLY_ARGS:
FUNCTION
Expand Down
1 change: 0 additions & 1 deletion tests/version/version-only.earth

This file was deleted.

Loading