test: add fluxtest harness #21074
test: add fluxtest harness #21074
Conversation
| config.HTTPD.FluxTestingEnabled = true | ||
|
|
||
| // NOTE: This will panic on failure. Not changing it for now. | ||
| e.s = tests.OpenServer(config) |
There was a problem hiding this comment.
I'm torn between this approach and spinning up a separate Influxd process for a real integration test.
There was a problem hiding this comment.
I'd vote for leaving it as-is for now, to mirror 2.x.
danxmoran
force-pushed
the
dm-backport-flux-test-harness-21072
branch
from
c0ee3e8 to
d26687e
Compare
| BucketID string `json:"bucketID,omitempty"` | ||
| } | ||
|
|
||
| func init() { |
There was a problem hiding this comment.
This logic was performing the same rewrite logic as the new FromStorageRule pushdown. I thought it'd be good to refactor into a 1:1 match with the 2.x code, so future back-/forward-ports have fewer differences to think through.
| @@ -10,6 +10,7 @@ | |||
| # 2: normal 32bit tests | |||
| # 3: tsi build | |||
| # count: print the number of test environments | |||
| # flux: run Flux e2e tests via the external test harness | |||
There was a problem hiding this comment.
I didn't add a new number for this because I wanted these tests to run in a separate CI job
There was a problem hiding this comment.
I like this - should we plan to move the rest of the tests to their own CI jobs?
There was a problem hiding this comment.
I would vote for yes (2.x is already there, it's nice to see more fine-grained reporting)
| @@ -70,7 +70,8 @@ type Server struct { | |||
| BindAddress string | |||
| Listener net.Listener | |||
|
|
|||
| Logger *zap.Logger | |||
| Logger *zap.Logger | |||
| MuxLogger *log.Logger | |||
There was a problem hiding this comment.
Pulled this up to the top level so we can redirect it during tests and not see a bunch of "failed to connect" logs from startup.
There was a problem hiding this comment.
Most of this is over my head, but I have two things to point out:
- Xenial Ubuntu (16.04) is almost EOL -- it might be a good idea to start moving dockerfiles to Bionic (18.04) or Focal (20.04)
- This isn't isolated to just you, but I'd like to see functions documented in the Godoc style (regular comments directly preceding the function definition)
| @@ -10368,7 +10369,7 @@ func runFluxBuiltinTest(t *testing.T, file *ast.File, u *url.URL, bucket string, | |||
|
|
|||
| inspectCalls := stdlib.TestingInspectCalls(pkg) | |||
| if len(inspectCalls.Body) == 0 { | |||
| t.Skip("No tests in builtin test package") | |||
| t.Skip("No tests in init test package") | |||
There was a problem hiding this comment.
Do we still need the EndToEnd tests built in if we're running externally?
There was a problem hiding this comment.
The Flux team said yes (for now), because they haven't fully reproduced the existing test-cases yet. I'll make a ticket for eventually deleting EndToEnd
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/python2.7 -u | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
We were getting away with using Python 2 because of the old Ubuntu base image. Luckily it seems we weren't using anything specific to v2
| @@ -0,0 +1,50 @@ | |||
| package universe_test | |||
There was a problem hiding this comment.
This test was in the Flux repo, but it was recently commented out (I assume because IDPE hasn't implemented the pushdown yet). Transferred it here so we're protected against regressions on this rewrite rule.
Closes #21072
The logic here is a mix of:
fluxtestharness code from 2.xTestFluxEndToEndTODO list:
etc/test-flux.shrun at alletc/test-flux.shpassetc/test-flux.shto CI