I think it's a good point that we may only want to run some tests once, not once per OS/Arch. However, I believe these particular tests are still limited in scope by the OS/arch of the host machine. The go/build package respects the build tags in files, so in theory, one run of this on Ubuntu could miss code that is +build windows. (we could give go/build different os/arch constraints, but then we're back to running this N times for N os/arch variants)
I'm certainly happy to have these tests run in verify.sh, but it sounds like they may need to be run per-OS testrun as well. On Sun, May 1, 2016 at 10:27 PM Andrew Wilkins <andrew.wilk...@canonical.com> wrote: > On Thu, Apr 28, 2016 at 11:48 AM Nate Finch <nate.fi...@canonical.com> > wrote: > >> Maybe we're not as far apart as I thought at first. >> > > >> My thought was that they'd live under github.com/juju/juju/devrules (or >> some other name) and therefore only get run during a full test run or if >> you run them there specifically. What is a full test run if not a test of >> all our code? These tests just happen to test all the code at once, rather >> than piece by piece. Combining with the other thread, if we also marked >> them as skipped under -short, you could easily still run go test ./... >> -short from the root of the juju repo and not incur the extra 16.5 seconds >> (gocheck has a nice feature where if you call c.Skip() in the SetUpSuite, >> it skips all the tests in the suite, which is particularly appropriate to >> these tests, since it's the SetUpSuite that takes all the time). >> > > I'm not opposed to using the Go testing framework in this instance, > because it makes most sense to write the analysis code in Go. That may not > always be the case, though, and I don't want to have a rule of "everything > as Go tests" that means we end up shoe-horning things. This is just > academic until we need something that doesn't live in the Go ecosystem, > though. > > Most importantly, I don't want to lose the ability to distinguish the > types of tests. As an example: where we run static analysis should never > matter, so we can cut a merge job short by performing all of the static > analysis checks up front. That doesn't matter much if we only gate merges > on running the tests on one Ubuntu series/arch; but what if we want to > start gating on Windows, CentOS, or additional architectures? It would not > make sense to run them all in parallel if they're all going to fail on the > static analysis tests. And then if we've run them up front, it would be > ideal to not have to run them on the individual test machines. > > So I think it would be OK to have a separate "devrules" package, or > whatever we want to call it. I would still like these tests to be run by > verify.sh, so we have one place to go to check that the source code is > healthy, without also running the unit tests or feature tests. If we have a > separate package like this, test tags are not really necessary in the short > term -- the distinction is made by separating the tests into their own > package. We could still mark them as short/long, but that's orthogonal to > separation-by-purpose. > > Cheers, > Andrew > > Mostly, I just didn't want them to live off in a separate repo or run with >> a separate tool. >> >> On Wed, Apr 27, 2016 at 11:39 PM Andrew Wilkins < >> andrew.wilk...@canonical.com> wrote: >> >>> On Thu, Apr 28, 2016 at 11:14 AM Nate Finch <nate.fi...@canonical.com> >>> wrote: >>> >>>> From the other thread: >>>> >>>> I wrote a test that parses the entire codebase under >>>> github.com/juju/juju to look for places where we're creating a new >>>> value of crypto/tls.Config instead of using the new helper function that I >>>> wrote that creates one with more secure defaults. It takes 16.5 seconds to >>>> run on my machine. There's not really any getting around the fact that >>>> parsing the whole tree takes a long time. >>>> >>>> What I *don't* want is to put these tests somewhere else which requires >>>> more thought/setup to run. So, no separate long-tests directory or >>>> anything. Keep the tests close to the code and run in the same way we run >>>> unit tests. >>>> >>> >>> The general answer to this belongs back in the other thread, but I agree >>> that long-running *unit* tests (if there should ever be such a thing) >>> should not be shunted off to another location. Keep the unit tests with the >>> unit. Integration tests are a different matter, because they cross multiple >>> units. Likewise, tests for project policies. >>> >>> Andrew's response: >>>> >>>> >>>> *The nature of the test is important here: it's not a test of Juju >>>> functionality, but a test to ensure that we don't accidentally use a TLS >>>> configuration that doesn't match our project-wide constraints. It's static >>>> analysis, using the test framework; and FWIW, the sort of thing that Lingo >>>> would be a good fit for.* >>>> >>>> *I'd suggest that we do organise things like this separately, and run >>>> them as part of the "scripts/verify.sh" script. This is the sort of test >>>> that you shouldn't need to run often, but I'd like us to gate merges on.* >>>> >>>> So, I don't really think the method of testing should determine where a >>>> test lives or how it is run. I could test the exact same things with a >>>> more common unit test - check the tls config we use when dialing the API is >>>> using tls 1.2, that it only uses these specific ciphersuites, etc. In >>>> fact, we have some unit tests that do just that, to verify that SSL is >>>> disabled. However, then we'd need to remember to write those same tests >>>> for every place we make a tls.Config. >>>> >>> >>> The method of testing is not particularly relevant; it's the *purpose* >>> that matters. You could probably use static analysis for a lot of our >>> units; it would be inappropriate, but they'd still be testing units, and so >>> should live with them. >>> >>> The point I was trying to make is that this is not a test of one unit, >>> but a policy that covers the entire codebase. You say that you don't want >>> to it put them "somewhere else", but it's not at all clear to me where you >>> think we *should* have them. >>> >>>> The thing I like about having this as part of the unit tests is that >>>> it's zero friction. They already gate landings. We can write them and run >>>> them them just like we write and run go tests 1000 times a day. They're >>>> not special. There's no other commands I need to remember to run, scripts >>>> I need to remember to set up. It's go test, end of story. >>>> >>> >>> Using the Go testing framework is fine. I only want to make sure we're >>> not slowing down the edit/test cycle by frequently testing things that are >>> infrequently going to change. It's the same deal as with integration tests; >>> there's a trade-off between the time spent and confidence level. >>> >>>> The comment about Lingo is valid, though I think we have room for both >>>> in our processes. Lingo, in my mind, is more appropriate at review-time, >>>> which allows us to write lingo rules that may not have 100% confidence. >>>> They can be strong suggestions rather than gating rules. The type of test >>>> I wrote should be a gating rule - there are no false positives. >>>> >>>> To give a little more context, I wrote the test as a suite, where you >>>> can add tests to hook into the code parsing, so we can trivially add more >>>> tests that use the full parsed code, while only incurring the 16.5 second >>>> parsing hit once for the entire suite. That doesn't really affect this >>>> discussion at all, but I figured people might appreciate that this could be >>>> extended for more than my one specific test. I certainly wouldn't advocate >>>> people writing new 17 seconds tests all over the place. >>>> >>> >>> That sounds lovely, thank you. >>> >>> Cheers, >>> Andrew >>> >>
-- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
