gravwell / gcfg

read INI-style configuration files into Go structs; supports user-defined types and subsections
https://gopkg.in/gcfg.v1
Other
0 stars 4 forks source link

Broken tests #2

Closed traetox closed 4 years ago

traetox commented 4 years ago

These tests appear to be wrong, some of these integers CLEARLY shouldn't be accepted.

Whats up here? Is this just a bad test? Did we break these?

kris@box:~/githubwork/gcfg$ go test ./...
ok      github.com/gravwell/gcfg        0.007s
ok      github.com/gravwell/gcfg/scanner        0.001s
ok      github.com/gravwell/gcfg/token  0.008s
--- FAIL: TestParseInt (0.00s)
    int_test.go:63: ParseInt(int, "0", IntMode(Dec)): pass; got 0, error <nil>
    int_test.go:63: ParseInt(int, "10", IntMode(Dec)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "-10", IntMode(Dec)): pass; got -10, error <nil>
    int_test.go:63: ParseInt(int, "x", IntMode(Dec)): pass; got 0, error failed to parse "x" as int: expected integer
    int_test.go:63: ParseInt(int, "0xa", IntMode(Hex)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "a", IntMode(Hex)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "10", IntMode(Hex)): pass; got 16, error <nil>
    int_test.go:63: ParseInt(int, "-0xa", IntMode(Hex)): pass; got -10, error <nil>
    int_test.go:54: ParseInt(int, "0x", IntMode(Hex)): fail; got error failed to parse "0x" as int: strconv.ParseInt: parsing "0x": invalid syntax, want ok
    int_test.go:54: ParseInt(int, "-0x", IntMode(Hex)): fail; got error failed to parse "-0x" as int: strconv.ParseInt: parsing "-0x": invalid syntax, want ok
    int_test.go:63: ParseInt(int, "-a", IntMode(Hex)): pass; got -10, error <nil>
    int_test.go:63: ParseInt(int, "-10", IntMode(Hex)): pass; got -16, error <nil>
    int_test.go:63: ParseInt(int, "x", IntMode(Hex)): pass; got 0, error failed to parse "x" as int: expected integer
    int_test.go:63: ParseInt(int, "10", IntMode(Oct)): pass; got 8, error <nil>
    int_test.go:63: ParseInt(int, "010", IntMode(Oct)): pass; got 8, error <nil>
    int_test.go:63: ParseInt(int, "-10", IntMode(Oct)): pass; got -8, error <nil>
    int_test.go:63: ParseInt(int, "-010", IntMode(Oct)): pass; got -8, error <nil>
    int_test.go:63: ParseInt(int, "10", IntMode(Dec|Hex)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "010", IntMode(Dec|Hex)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "0x10", IntMode(Dec|Hex)): pass; got 16, error <nil>
    int_test.go:63: ParseInt(int, "10", IntMode(Dec|Oct)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "010", IntMode(Dec|Oct)): pass; got 8, error <nil>
    int_test.go:63: ParseInt(int, "0x10", IntMode(Dec|Oct)): pass; got 0, error failed to parse "0x10" as int: extra characters "x10"
    int_test.go:63: ParseInt(int, "10", IntMode(Hex|Oct)): pass; got 0, error ambiguous integer value; must include '0' prefix
    int_test.go:63: ParseInt(int, "010", IntMode(Hex|Oct)): pass; got 8, error <nil>
    int_test.go:63: ParseInt(int, "0x10", IntMode(Hex|Oct)): pass; got 16, error <nil>
    int_test.go:63: ParseInt(int, "10", IntMode(Dec|Hex|Oct)): pass; got 10, error <nil>
    int_test.go:63: ParseInt(int, "010", IntMode(Dec|Hex|Oct)): pass; got 8, error <nil>
    int_test.go:63: ParseInt(int, "0x10", IntMode(Dec|Hex|Oct)): pass; got 16, error <nil>
--- FAIL: TestScanFully (0.00s)
    scan_test.go:32: ScanFully(*int, "a", 'v') = failed to parse "a" as int: expected integer; *ptr==0
    scan_test.go:23: ScanFully(*int, "0x", 'v'): want ok, got error failed to parse "0x" as int: strconv.ParseInt: parsing "0x": invalid syntax
    scan_test.go:32: ScanFully(*int, "0x", 'd') = failed to parse "0x" as int: extra characters "x"; *ptr==0
FAIL
FAIL    github.com/gravwell/gcfg/types  0.002s
FAIL
djfritz commented 4 years ago

So scanf definitely can parse things like "0x" as 0, as in:

package main

import (
    "fmt"
    "os"
    "strings"
)

func main() {
    var i int

    r := strings.NewReader("0x")

    n, err := fmt.Fscanf(r, "%x", &i)
    if err != nil {
        fmt.Fprintf(os.Stderr, "Fscanf: %v\n", err)
    }
    fmt.Println(i)
    fmt.Println(n)
}

But strconv.ParseInt doesn't follow scanf's behavior, as in:

package main

import (
    "fmt"
    "strconv"
)

func main() {
    v := "0x"
    s, err := strconv.ParseInt(v, 0, 64)
    if err != nil {
        fmt.Println(err)
    }
    fmt.Printf("%T, %v\n", s, s)

}

The latter program will error with "0x invalid syntax".

So these are broken tests/assertions about how to parse thing. Did we write these tests or did these come from upstream?

djfritz commented 4 years ago

I should add that the parser in gcfg is using strconv.ParseInt

traetox commented 4 years ago

I did not write those tests. Are they broken upstream?

On Fri, Jun 12, 2020, 12:31 PM David Fritz notifications@github.com wrote:

I should add that the parser in gcfg is using strconv.ParseInt

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gravwell/gcfg/issues/2#issuecomment-643424519, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKD2OFZQ7WAK4VA25HBXO3RWJX7BANCNFSM4N4Q3XBA .

djfritz commented 4 years ago

They must be - there's a comment in the tests asserting that scanf works on empty prefixes (0x, 0b, ...) which it does (even C stdlib does this), but then they use strconv to parse everything, which doesn't at all follow the scanf behavior...

djfritz commented 4 years ago

So either the test is wrong or the expectation of their implementation is wrong. I patched for the test...