[
https://issues.apache.org/jira/browse/HDDS-5097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Rakesh Radhakrishnan reassigned HDDS-5097:
------------------------------------------
Assignee: Rakesh Radhakrishnan
> [FSO] Cleanup integration tests and reduce the execution time
> --------------------------------------------------------------
>
> Key: HDDS-5097
> URL: https://issues.apache.org/jira/browse/HDDS-5097
> Project: Apache Ozone
> Issue Type: Sub-task
> Reporter: Marton Elek
> Assignee: Rakesh Radhakrishnan
> Priority: Blocker
> Labels: pull-request-available
>
> 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.
> HDDS-2939 branch build time has been increased significantly. The problem
> with acceptance tests are tracked with HDDS-5093.
> 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:
> {code}
> tmp % grep -e 'Tests run: .* in org' "20" | awk -F '[,:\-]' '{print $6,
> $8, $10, $12, $14, $15}' | sed 's/s in //g' | sort -n -k5 -r | head
> awk: warning: escape sequence `\-' treated as plain `-'
> 104 0 0 0 478.951 org.apache.hadoop.fs.ozone.TestRootedOzoneFileSystem
> 88 0 0 0 458.703 org.apache.hadoop.fs.ozone.TestOzoneFileSystem
> 18 0 0 2 436.521 org.apache.hadoop.fs.ozone.TestOzoneFileInterfaces
> 6 0 0 0 152.808
> org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate
> 4 0 0 0 108.191 org.apache.hadoop.hdds.scm.pipeline.TestPipelineClose
> 1 0 0 0 88.905 org.apache.hadoop.hdds.scm.pipeline.TestNodeFailure
> 2 0 0 0 74.192 org.apache.hadoop.fs.ozone.TestOzoneFsHAURLs
> 19 0 0 0 58.2
> org.apache.hadoop.fs.ozone.contract.rooted.ITestRootedOzoneContractGetFileStatus
> 8 0 0 0 57.063
> org.apache.hadoop.fs.ozone.contract.rooted.ITestRootedOzoneContractDistCp
> 8 0 0 0 54.612
> org.apache.hadoop.fs.ozone.contract.ITestOzoneContractDistCp
> tmp % grep -e 'Tests run: .* in org' "18" | awk -F '[,:\-]' '{print $6,
> $8, $10, $12, $14, $15}' | sed 's/s in //g' | sort -n -k5 -r | head
> awk: warning: escape sequence `\-' treated as plain `-'
> 128 0 0 0 509.388 org.apache.hadoop.fs.ozone.TestRootedOzoneFileSystem
> 132 0 0 0 508.585 org.apache.hadoop.fs.ozone.TestOzoneFileSystem
> 140 0 0 8 479.578 org.apache.hadoop.fs.ozone.TestOzoneFileSystemV1
> 18 0 0 2 475.686 org.apache.hadoop.fs.ozone.TestOzoneFileInterfaces
> 128 0 0 0 470.619 org.apache.hadoop.fs.ozone.TestRootedOzoneFileSystemV1
> 18 0 0 6 329.812 org.apache.hadoop.fs.ozone.TestOzoneFileInterfacesV1
> 5 0 0 0 226.093
> org.apache.hadoop.fs.ozone.TestDirectoryDeletingServiceWithFSOBucket
> 6 0 0 0 171.861
> org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate
> 4 0 0 0 139.951 org.apache.hadoop.hdds.scm.pipeline.TestPipelineClose
> 3 0 0 0 82.128 org.apache.hadoop.fs.ozone.TestOzoneFileSystemPrefixParser
> {code}
> 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.
> Suggestions:
> 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...)
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
