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

Put the github.com/BurntSushi/toml-test in internal/ #313

Merged
merged 1 commit into from Aug 5, 2021
Merged

Put the github.com/BurntSushi/toml-test in internal/ #313

merged 1 commit into from Aug 5, 2021

Commits on Aug 5, 2021

  1. Put the github.com/BurntSushi/toml-test in internal/

    The problem is as follows:
    
    - BurntSushi/toml is a TOML library.
    
    - BurntSushi/toml-test is a generic language-agnostic TOML testing
      framework that uses this toml library.
    
    - toml-test is based on a CLI interface. This works well, but for this
      library (since it's written in Go) it's actually quite a bit more
      convenient to integrate this in "go test"; it's more flexible, easier,
      and gives us stuff like code coverage.
    
    - This introduces a bit of a weird dependency scenario:
    
      1. toml depends on toml-test
      2. toml-test depends on toml
      3. Because toml-test is only referred to from the toml_test package,
         this should actually be fine (similar to how you can use the _test
         package to work around other cyclic dependency issues).
    
    - For v0.4.0 the dependencies aren't quite "correct"; it depends on
      toml-test facb9eccd4da, which in turn depends on toml 20a94d6.
      This isn't *necessarily* a problem as such as this can be resolved,
      but commit 20a94d6 on toml is not referenced at all and orphaned:
      I updated the tests in sync with the feature I was working on, and
      then later rebased the lot it never ended up in the master branch
    
      All of this happens to work when using the standard proxy.golang.org
      GOPROXY because it has that commit in the cache, but right not it
      doesn't work with GOPROXY=direct as that doesn't have any cache (and a
      private GOPROXY won't either).
    
    This should be fixed by just tagging the current master as v0.4.1, as
    that now refers to toml-test v1.0.0, which refers to toml v0.4.0. I'm
    not super-happpy with that solution as such, because I can see it break
    in the future.
    
    So instead, just copy the toml-test package to internal/toml-test here
    to get rid of any cyclic module dependency.
    
    More details can be found in toml-lang/toml-test#74
    
    One downside is that updating this is a bit awkward now. That's okay for
    the time being and only affects me, and this doesn't need updating all
    that often anyway.
    
    Another downside is:
    
    	[~c/toml](master)% du -hd1 internal/
    	1.7M    internal/toml-test
    	12K     internal/tag
    	1.7M    internal/
    
    1.7M is kind of a lot. But then again, it's required for running the
    tests, and all of it is actual test cases. They would be in *_test.go
    files otherwise anyway.
    
    The version in the go.mod is updated because without it:
    
    	[~c/toml](dep)% go test ./...
    	# github.com/BurntSushi/toml/internal/toml-test [github.com/BurntSushi/toml.test]
    	internal/toml-test/runner.go:27:3: go:embed requires go1.16 or later (-lang was set to go1.13; check go.mod)
    	# github.com/BurntSushi/toml/internal/toml-test
    	internal/toml-test/runner.go:27:3: go:embed requires go1.16 or later (-lang was set to go1.13; check go.mod)
    	FAIL    github.com/BurntSushi/toml [build failed]
    	?       github.com/BurntSushi/toml/cmd/toml-test-decoder        [no test files]
    	?       github.com/BurntSushi/toml/cmd/toml-test-encoder        [no test files]
    	?       github.com/BurntSushi/toml/cmd/tomlv    [no test files]
    	?       github.com/BurntSushi/toml/internal     [no test files]
    	?       github.com/BurntSushi/toml/internal/tag [no test files]
    
    It doesn't "see" that these files are protected by a go1.16 build tag.
    
    It should still work for older versions of Go though, just running these
    tests won't, but that was already the case (toml_test.go has a go1.16
    build tag). I also had to add a build tag to the Go files in
    toml-test.go, since it won't be able to find the embed and io/fs imports
    on older versions of Go.
    
    This also adds GOPROXY=direct in the CI. There aren't any dependencies
    in go.mod now, but this avoids depending on the peculiarities of
    proxy.golang.org, and is probably a good idea for most Go CIs.
    arp242 committed Aug 5, 2021
    Configuration menu
    Copy the full SHA
    4f1f359 View commit details
    Browse the repository at this point in the history