Experiment: Run static checks and postgres tests before all else #27971
Conversation
This should allow for faster failure / iteration time on non-hosted runners.
dstandish
added
the
use public runners
dstandish
removed
the
use public runners
| @@ -778,8 +780,8 @@ jobs: | |||
| needs: [build-info, wait-for-ci-images] | |||
| strategy: | |||
| matrix: | |||
| python-version: "${{fromJson(needs.build-info.outputs.python-versions)}}" | |||
| postgres-version: "${{fromJson(needs.build-info.outputs.postgres-versions)}}" | |||
| python-version: ["3.7"] | |||
There was a problem hiding this comment.
This is a bit dangerous and not needed. For most cases (as of last week) the exclude matrices are only selecting "representative" combos for tests - so you do not save by hard-conding those I think.
There was a problem hiding this comment.
yeah both this and putting test-postgres everywhere was just for testing
| @@ -366,7 +368,7 @@ jobs: | |||
| ${{needs.build-info.outputs.build-job-description}} PROD images | |||
| ${{needs.build-info.outputs.all-python-versions-list-as-string}} | |||
| runs-on: "${{needs.build-info.outputs.runs-on}}" | |||
| needs: [build-info, build-ci-images] | |||
| needs: [build-info, build-ci-images, tests-postgres] | |||
There was a problem hiding this comment.
I suggest to introduce an empty "smoke-tests" job that will be dependent o tests-postgres and use dependency like.
[build-info, smoke-tests]
This will be much more "generic". Unlike build-info (which is used in "needs.build-info", we do not need build-ci-images if "smoke-tests" (or "tests-postgres" depend on it already. Unfortunately github actions dependencies are not transitive and if we need "build-info" outputs, in needs clause we need to add it here.
There was a problem hiding this comment.
i agree, if we want to do something like that... this would be the way to go.
|
I am not sure if this is helpful, For non-hosted runners it should be no difference. We already have 900 runners by the ASF and we should have no more queueing here (at least we stopped seeing those) - so this is pretty ok to run all test types in parallel. And it will introduce significant "sequential" delays in case of non-postgres failures I am afraid. Rather than running tests in parallel, it will run them sequentially so "feedback time" will be biggeer. When optimising CI builds there are two aspects:
Feedback time is very important - the time between push and "result" should be as short as possible. Putting sequential dependency between tests increases feedback time, but saves cost (assuming job queue is infinite - and with 900 jobs in the ASF as of 3 weeks, for now - until ASF usage grows significantly it almost is - we used to have 300) . Cost does not matter - it is essentially free. |
|
yeah i agree. i was actually just about to close the PR. |
|
it would help if there were too many PRs competing for too few runners -- then we quickly get a quick read on the tests most likely to fail. but that doesn't really seem to be the case here. |
This
shouldmay (?) allow for faster failure / iteration time on non-hosted runners.