Move and convert all AWS example dags to system tests

[vincbeck]

Per discussion in #22438, we decided to move all example dags in folder airflow/providers/amazon/aws/example_dags/ to the system test folder tests/system/providers/amazon/aws/.

Sorry for the length of this PR but the essential modifications are:

• Moving files from airflow/providers/amazon/aws/example_dags/ to tests/system/providers/amazon/aws/
• Convertir the example DAGs to system test format

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[vincbeck]

I removed this test because, I think, it no longer make sense. Please let me know if any concerns

[potiuk]

@o-nikolas @eladkal ?

[o-nikolas]

I worry a little about:

1. The correctness of some of the changes. They looked good when I scanned them by eye, but it's always hard to say for sure until you run the tests (which I assume you couldn't for most of these since it would require the setup of many external resources and services)
2. Are people going to be confused why we don't run many of these tests? So far we only have migrated examples-->tests that we are committed to running in our test setup. We won't be running many of the tests in this PR since we don't have the external accounts and resources required to run them (many of the transfers are a good example of this).

[vandonr-amz]

I agree with Niko, I find it confusing to mix together examples that are actual test being run and dags that are just sitting there without being tested...
What's the problem with having the tests that we run in tests/system, and the ones that we don't in example_dags ?

[eladkal]

The current state where we have examples speared throughout the project is not intuitive. Which is why I raised this as an issue #22438 (comment)

[shubham22]

I'm with Elad on this. Having just a couple of examples left in examples folder is rather confusing for user's pov. Providing all examples / system tests in one place makes it easier to reference other tests as examples when writing a DAG.

Are people going to be confused why we don't run many of these tests?

I think this is why we added the reasoning on System test dashboard. We can add it in System test rst as well.

[potiuk]

I'm with Elad on this. Having just a couple of examples left in examples folder is rather confusing for user's pov. Providing all examples / system tests in one place makes it easier to reference other tests as examples when writing a DAG.

Are people going to be confused why we don't run many of these tests? So far we only have migrated examples-->tests that we are committed to running in our test setup. We won't be running many of the tests in this PR since we don't have the external accounts and resources required to run them (many of the transfers are a good example of this).

Yes it's a valid worry. But maybe we could make it clearer by slight modification of the pytest code so that it will raise Skip exception for those tests unless we setup some variable (RUN_EXPERIMENTAL_TESTS and providing skip reason). This will be visible in the code, as well as in your dashboard you could run whole folder, and possibly even report number of skipped tests. I think this both removes the confusion and makes it clear which tests are "tested".

[eladkal]

Yes it's a valid worry. But maybe we could make it clearer by slight modification of the pytest code so that it will raise Skip exception for those tests unless we setup some variable (RUN_EXPERIMENTAL_TESTS and providing skip reason). This will be visible in the code, as well as in your dashboard you could run whole folder, and possibly even report number of skipped tests. I think this both removes the confusion and makes it clear which tests are "tested".

+1
Ny initial tought was lets make all examples in the system tests structure but touse we know that are not working/have issues we mark them as quarantined.

[vincbeck]

I'm with Elad on this. Having just a couple of examples left in examples folder is rather confusing for user's pov. Providing all examples / system tests in one place makes it easier to reference other tests as examples when writing a DAG.

Are people going to be confused why we don't run many of these tests? So far we only have migrated examples-->tests that we are committed to running in our test setup. We won't be running many of the tests in this PR since we don't have the external accounts and resources required to run them (many of the transfers are a good example of this).

Yes it's a valid worry. But maybe we could make it clearer by slight modification of the pytest code so that it will raise Skip exception for those tests unless we setup some variable (RUN_EXPERIMENTAL_TESTS and providing skip reason). This will be visible in the code, as well as in your dashboard you could run whole folder, and possibly even report number of skipped tests. I think this both removes the confusion and makes it clear which tests are "tested".

My concern with this approach is we would end up having 2 categories of system tests: experimental and non experimental ones. IMHO, we should not have such categorization, users are welcome to run them if they want regardless of whether AWS run them as part of their health dashboard. Moreover, forcing users to have the flag RUN_EXPERIMENTAL_TESTS on to run these tests would make it more difficult for users to run them (or would incentivize them to NOT run them).

We made some recent changes in the health dashboard so that system tests which are not run are listed in the dashboard but with no information (since they are not run). See dashboard. If we go with this change, then all example DAGs would show up in the dashboard and styled like example_dms or example_emr_notebook_execution. To me this makes sense. We can go further and move system tests which are not run to the bottom of the dashboard.

WDYT?

[o-nikolas]

I'm with Elad on this. Having just a couple of examples left in examples folder is rather confusing for user's pov. Providing all examples / system tests in one place makes it easier to reference other tests as examples when writing a DAG.

Are people going to be confused why we don't run many of these tests? So far we only have migrated examples-->tests that we are committed to running in our test setup. We won't be running many of the tests in this PR since we don't have the external accounts and resources required to run them (many of the transfers are a good example of this).

Yes it's a valid worry. But maybe we could make it clearer by slight modification of the pytest code so that it will raise Skip exception for those tests unless we setup some variable (RUN_EXPERIMENTAL_TESTS and providing skip reason). This will be visible in the code, as well as in your dashboard you could run whole folder, and possibly even report number of skipped tests. I think this both removes the confusion and makes it clear which tests are "tested".

My concern with this approach is we would end up having 2 categories of system tests: experimental and non experimental ones. IMHO, we should not have such categorization, users are welcome to run them if they want regardless of whether AWS run them as part of their health dashboard. Moreover, forcing users to have the flag RUN_EXPERIMENTAL_TESTS on to run these tests would make it more difficult for users to run them (or would incentivize them to NOT run them).

We made some recent changes in the health dashboard so that system tests which are not run are listed in the dashboard but with no information (since they are not run). See dashboard. If we go with this change, then all example DAGs would show up in the dashboard and styled like example_dms or example_emr_notebook_execution. To me this makes sense. We can go further and move system tests which are not run to the bottom of the dashboard.

WDYT?

Great discussion everyone! It appears I'm out voted and most folks would like to see them moved. I'm happy to commit to that approach :)

And I think this is a fair counter point from Vincent, maybe we shouldn't over complicate things. We can try first with the more simplified approach you have now, this is not a one-way door, if it does cause confusion with users we can make some adjustments/optimizations later.

[potiuk]

Yep. Let's merge it now.