Affects Version/s: None
Fix Version/s: None
Apache organization has a shared, organization-level resource limit for Github actions. All the projects should use the available minutes fairly to avoid blocking the build of other Apache projects.
This is discussed recently here: https://lists.apache.org/thread.html/r48d079eeff292254db22705c8ef8618f87ff7adc68d56c4e5d0b4105%40%3Cbuilds.apache.org%3E, but same mailing list has older threads, too.
But integration tests are also increased from ~56 minutes to 94 minutes (~158%)
We need to avoid such increase unless we have very good reason to have it.
If we check the execution time of the tests on the branch and the master we can see the problem in more details:
It turned out that a lot of tests just copied with V1 prefix with small modification (or original test extended with V1 which means the methods of the old and new tests are executed) instead of improving the original test to cover both of the cases (simple/prefix-ed).
Also: the same functionality seems to be tested on multiple levels (FileSystemInterface, OMRequest, acceptance test...)
This copy can make the maintenance of the tests slightly harder, and the V1 prefix is quite meaningless and confusing.
From the legendary "Clean code" book:
>Programmers create problems for themselves when they write code solely to satisfy a compiler or interpreter. For example, because you can’t use the same name to refer to two different things in the same scope, you might be tempted to change one name in an arbitrary way. Sometimes this is done by misspelling one, leading to the surprising situation where correcting spelling errors leads to an inability to compile.2
> [...] It is not sufficient to add number series or noise words, even though the compiler is satisfied. If names must be different, then they should also mean something different.
> Number-series naming (a1, a2, .. aN) is the opposite of intentional naming. Such names are not disinformative—they are noninformative; they provide no clue to the author’s intention....
I totally agree with this section, I think we should avoid using V1 prefixes everywhere.
1. We should minimize the additional build time when possible.
2. Similar to
HDDS-5093 instead of creating a new dimension to the parametrized tests (which double the execution time) we should carefully select the meaningful sets of parameters. (When we have 3 parameters for a unit test with 2 values for each it's already 8 possible executions. Execution all of them with and without prefix may not be required, it seems to be better to select representative parameter sets and use only them).
3. The V1 prefixes should be avoided. When possible we need to modify the original unit tests with adding additional parameters. It makes it easier to maintain the unit tests, and it requires less code and (together with the previous point) less time
4. We don't need to repeat ALL the tests IMHO. For example if we modified only the rename part of the OzoneFileSystem, I assume that it's enough to test that part with/without prefix (especially as the prefix handling is already tested with testing om requests...)