search

google / cel-spec

Common Expression Language -- specification and binary representation
https://cel.dev
Apache License 2.0
2.88k stars 225 forks source link

Test failure in tests/simple: Go 1.13+: one shall not call flag.Parse() in init #176

Open eclipseo opened 3 years ago

eclipseo commented 3 years ago

Testing tests/simple fails with:

      testing: github.com/google/cel-spec
github.com/google/cel-spec/tests/simple
flag provided but not defined: -test.timeout
Usage of /tmp/go-build610373726/b001/simple.test:
  -check_server string
        path to command for check server
  -checked_only
        skip tests which skip type checking
  -eval_server string
        path to command for eval server
  -parse_server string
        path to command for parse server
  -server string
        path to binary for server when no phase-specific server defined
  -skip_check
        force skipping the check phase
  -skip_test value
        name(s) of tests to skip. can be set multiple times. to skip the following tests: f1/s1/t1, f1/s1/t2, f1/s2/*, f2/s3/t3, you give the arguments --skip_test=f1/s1/t1,t2;s2 --skip_test=f2/s3/t3
exit status 2
FAIL    github.com/google/cel-spec/tests/simple 0.013s

See bug golang/go#33475 and bug golang/go#31859 for the rationale

To be clear, we are talking only about flag.Parse here, not flag.*. The flag package is designed so that multiple packages can define flags, without those packages being aware of each other. It's essential that flag.Parse be called only after all flags are defined, meaning only after all packages are initialized. The normal practice is that the main package calls flag.Parse, because the main package is the only package that is sure to run after all packages are initialized.

Your program may well be designed so that it is OK to call flag.Parse in an init function of some non-main package. But most programs do not and cannot work that way. So I am questioning how much we should twist the testing package to support that use case. Of course if the change to the testing package is simple, then that is fine. But I would not like to see a lot of complexity added to support a very unusual use case.