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

matchN: inconsistent reporting of the full set of validation errors #3388

Closed
jpluscplusm opened this issue Aug 22, 2024 · 4 comments
Closed
Labels
evaluator evalv3-win Issues resolved by switching from evalv2 to evalv3 NeedsInvestigation

Comments

@jpluscplusm
Copy link
Collaborator

jpluscplusm commented Aug 22, 2024

What version of CUE are you using (cue version)?

$ cue version
cue version v0.11.0-0.dev.0.20240822081630-a20e523c2059

go version go1.23.0
      -buildmode exe
       -compiler gc
  DefaultGODEBUG asynctimerchan=1,gotypesalias=0,httpservecontentkeepheaders=1,tls3des=1,tlskyber=0,x509keypairleaf=0,x509negativeserial=1
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v1
cue.lang.version v0.11.0

Does this issue reproduce with the latest stable release?

n/a (function is not available in latest, v0.10.0)

What did you do?

! exec cue vet
cmp stderr stderr.golden

-- f.cue --
package p

A: 42
A: matchN(0, [int])
A: matchN(0, [string, number])
A: matchN(0, [42, >100, >1000])

B: 42
B: matchN(1, [int, >10])
B: matchN(1, [string, >100])

-- stderr.golden --
A: invalid value 42 (does not satisfy matchN(0, [int])): 1 matched, expected 0:
    ./f.cue:4:4
    ./f.cue:3:4
    ./f.cue:4:11
    ./f.cue:5:4
    ./f.cue:6:4
A: invalid value 42 (does not satisfy matchN(0, [string,number])): 1 matched, expected 0:
    ./f.cue:5:4
    ./f.cue:3:4
    ./f.cue:4:4
    ./f.cue:5:11
    ./f.cue:6:4
A: invalid value 42 (does not satisfy matchN(0, [42,>100,>1000])): 1 matched, expected 0:
    ./f.cue:6:4
    ./f.cue:3:4
    ./f.cue:4:4
    ./f.cue:5:4
    ./f.cue:6:11
B: invalid value 42 (does not satisfy matchN(1, [int,>10])): 2 matched, expected 1:
    ./f.cue:9:4
    ./f.cue:8:4
    ./f.cue:9:11
B: invalid value 42 (does not satisfy matchN(1, [string,>100])): 0 matched, expected 1:
    ./f.cue:10:4
    ./f.cue:8:4
    ./f.cue:9:4
    ./f.cue:10:11

Repro from @myitcv in #3388 (comment)

What did you expect to see?

A passing test.

What did you see instead?

> ! exec cue vet
[stderr]
A: invalid value 42 (does not satisfy matchN(0, [int])): 1 matched, expected 0:
    ./f.cue:4:4
    ./f.cue:3:4
    ./f.cue:4:11
    ./f.cue:5:4
    ./f.cue:6:4
A: invalid value 42 (does not satisfy matchN(0, [string,number])): 1 matched, expected 0:
    ./f.cue:5:4
    ./f.cue:3:4
    ./f.cue:4:4
    ./f.cue:5:11
    ./f.cue:6:4
A: invalid value 42 (does not satisfy matchN(0, [42,>100,>1000])): 1 matched, expected 0:
    ./f.cue:6:4
    ./f.cue:3:4
    ./f.cue:4:4
    ./f.cue:5:4
    ./f.cue:6:11
B: invalid value 42 (does not satisfy matchN(1, [string,>100])): 0 matched, expected 1:
    ./f.cue:10:4
    ./f.cue:8:4
    ./f.cue:9:4
    ./f.cue:10:11
[exit status 1]
> cmp stderr stderr.golden
diff stderr stderr.golden
--- stderr
+++ stderr.golden
@@ -16,6 +16,10 @@
     ./f.cue:4:4
     ./f.cue:5:4
     ./f.cue:6:11
+B: invalid value 42 (does not satisfy matchN(1, [int,>10])): 2 matched, expected 1:
+    ./f.cue:9:4
+    ./f.cue:8:4
+    ./f.cue:9:11
 B: invalid value 42 (does not satisfy matchN(1, [string,>100])): 0 matched, expected 1:
     ./f.cue:10:4
     ./f.cue:8:4

FAIL: /tmp/testscript912524492/matchN.inconsistentErrorReporting.txtar/script.txtar:2: stderr and stderr.golden differ
@jpluscplusm jpluscplusm added NeedsInvestigation Triage Requires triage/attention labels Aug 22, 2024
@myitcv

This comment was marked as resolved.

@myitcv myitcv removed the Triage Requires triage/attention label Aug 22, 2024
@mpvl
Copy link
Member

mpvl commented Aug 22, 2024

Observation: v3 does the right thing here.

@mpvl mpvl added the evalv3-win Issues resolved by switching from evalv2 to evalv3 label Aug 22, 2024
@mvdan
Copy link
Member

mvdan commented Oct 9, 2024

This appears to be fixed for both the old and new evaluators now; the testscript below passes:

env CUE_EXPERIMENT=evalv3=0
! exec cue vet
cmp stderr stderr.golden

env CUE_EXPERIMENT=evalv3=1
! exec cue vet
cmp stderr stderr.golden

-- f.cue --
package p

A: 42
A: matchN(0, [int])
A: matchN(0, [string, number])
A: matchN(0, [42, >100, >1000])

B: 42
B: matchN(1, [int, >10])
B: matchN(1, [string, >100])

-- stderr.golden --
A: invalid value 42 (does not satisfy matchN(0, [int])): 1 matched, expected 0:
    ./f.cue:4:4
    ./f.cue:3:4
    ./f.cue:4:11
    ./f.cue:5:4
    ./f.cue:6:4
A: invalid value 42 (does not satisfy matchN(0, [string,number])): 1 matched, expected 0:
    ./f.cue:5:4
    ./f.cue:3:4
    ./f.cue:4:4
    ./f.cue:5:11
    ./f.cue:6:4
A: invalid value 42 (does not satisfy matchN(0, [42,>100,>1000])): 1 matched, expected 0:
    ./f.cue:6:4
    ./f.cue:3:4
    ./f.cue:4:4
    ./f.cue:5:4
    ./f.cue:6:11
B: invalid value 42 (does not satisfy matchN(1, [int,>10])): 2 matched, expected 1:
    ./f.cue:9:4
    ./f.cue:8:4
    ./f.cue:9:11
    ./f.cue:10:4
B: invalid value 42 (does not satisfy matchN(1, [string,>100])): 0 matched, expected 1:
    ./f.cue:10:4
    ./f.cue:8:4
    ./f.cue:9:4
    ./f.cue:10:11

I bisected the fix to https://review.gerrithub.io/c/cue-lang/cue/+/1202211. Since that change already included plenty of test changes, and this bug only affected the old evaluator in the first place, I'm going to mark this issue as resolved now without adding an extra test to the repository.

@mvdan mvdan closed this as completed Oct 9, 2024
@mvdan
Copy link
Member

mvdan commented Oct 9, 2024

Also, the bisection above would make this issue a duplicate of #3418, which makes complete sense. There, @rogpeppe said that a matchN error showed up on evalv3 but not on evalv2, which matches Jonathan's behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluator evalv3-win Issues resolved by switching from evalv2 to evalv3 NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

4 participants