potiuk commented on PR #35160: URL: https://github.com/apache/airflow/pull/35160#issuecomment-1786093658
> One thought. I am wondering if we should not be opinionated here regarding `pytest-xdist`. In this PR you leave the choice to the user to use `pytest-xdist` or not. But does it make sense to run `pytest-xdist` with DB tests? It seems not. Why someone running non DB test would not want to run tests with `pytest-xdist`? I might miss some hardware/capacity requirements from `pytest-xdist` here. In other words: should not we enable `pytest-xdist` by default when running non DB tests and having an option to disable it. Should `pytest-xdist` parameter be just ignored when running DB tests? > > Edit: I refer to Breeze environment I wanted it to be less opinionated and have individual switches to be independent. Generally speaking we do not know if the tests are going to be DB or Non-DB unless we specify the --run-only-db-tests or --skip-db-tests. Those switches are selecting DB/Non-DB tests. But yeah generally speaking I think we can get a few combinations that make no sense and error them out. Re default values - let met think about it. We have basically a choice of: * make flags straightforward - i.e. when you specify them, you get what you want, otherwise you have fixed default - always the same * or make defaults depend on the context - they guess what you want based on other parameters I have a feeling the latter is a bit too "magical". It would also necessitate the flag to be `--xdist/--no-xdist` if we want to disable xdist when it's guessed that it should be enabled (sometimes we would not want to run non-db tests with xdist- when we debug xdist problem and the like). So having an `on/off` --xdist/--no-xdist with variable default, seems a bit too complex. I'd rather prefer to just error out if the combination is invalid. But I can do something else - I can add "All-NonDB" test type that will be similar to "All-Quarantined" - in the sense that it will imply `--use-xdist` and `--skip-db-tests` - and this can be used as shorthand (and in Breeze) to run Non-DB tests. That would keep simplicity and static defaults of the flags, and flexibility of running what you want, while --test-type "All-NonDB" will be a convenient way to run all non-DB tests with maximum parallelism. WDYT? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
