1*9880d681SAndroid Build Coastguard Worker================ 2*9880d681SAndroid Build Coastguard Worker lit TODO Items 3*9880d681SAndroid Build Coastguard Worker================ 4*9880d681SAndroid Build Coastguard Worker 5*9880d681SAndroid Build Coastguard WorkerInfrastructure 6*9880d681SAndroid Build Coastguard Worker============== 7*9880d681SAndroid Build Coastguard Worker 8*9880d681SAndroid Build Coastguard Worker1. Change to always load suites, then resolve command line arguments? 9*9880d681SAndroid Build Coastguard Worker 10*9880d681SAndroid Build Coastguard Worker Currently we expect each input argument to be a path on disk; we do a 11*9880d681SAndroid Build Coastguard Worker recursive search to find the test suite for each item, but then we only do a 12*9880d681SAndroid Build Coastguard Worker local search based at the input path to find tests. Additionally, for any path 13*9880d681SAndroid Build Coastguard Worker that matches a file on disk we explicitly construct a test instance (bypassing 14*9880d681SAndroid Build Coastguard Worker the formats on discovery implementation). 15*9880d681SAndroid Build Coastguard Worker 16*9880d681SAndroid Build Coastguard Worker This has a couple problems: 17*9880d681SAndroid Build Coastguard Worker 18*9880d681SAndroid Build Coastguard Worker * The test format doesn't have control over the test instances that result 19*9880d681SAndroid Build Coastguard Worker from file paths. 20*9880d681SAndroid Build Coastguard Worker 21*9880d681SAndroid Build Coastguard Worker * It isn't possible to specify virtual tests as inputs. For example, it is not 22*9880d681SAndroid Build Coastguard Worker possible to specify an individual subtest to run with the googletest format. 23*9880d681SAndroid Build Coastguard Worker 24*9880d681SAndroid Build Coastguard Worker * The test format doesn't have full control over the discovery of tests in 25*9880d681SAndroid Build Coastguard Worker subdirectories. 26*9880d681SAndroid Build Coastguard Worker 27*9880d681SAndroid Build Coastguard Worker Instead, we should move to a model whereby first all of the input specifiers 28*9880d681SAndroid Build Coastguard Worker are resolved to test suites, and then the resolution of the input specifier is 29*9880d681SAndroid Build Coastguard Worker delegated to each test suite. This could take a couple forms: 30*9880d681SAndroid Build Coastguard Worker 31*9880d681SAndroid Build Coastguard Worker * We could resolve to test suites, then fully load each test suite, then have 32*9880d681SAndroid Build Coastguard Worker a fixed process to map input specifiers to tests in the test suite 33*9880d681SAndroid Build Coastguard Worker (presumably based on path-in-suite derivations). This has the benefit of 34*9880d681SAndroid Build Coastguard Worker being consistent across all test formats, but the downside of requiring 35*9880d681SAndroid Build Coastguard Worker loading the entire test suite. 36*9880d681SAndroid Build Coastguard Worker 37*9880d681SAndroid Build Coastguard Worker * We could delegate all of the resolution of specifiers to the test 38*9880d681SAndroid Build Coastguard Worker suite. This would allow formats that anticipate large test suites to manage 39*9880d681SAndroid Build Coastguard Worker their own resolution for better performance. We could provide a default 40*9880d681SAndroid Build Coastguard Worker resolution strategy that was similar to what we do now (start at subpaths 41*9880d681SAndroid Build Coastguard Worker for directories, but allow the test format control over what happens for 42*9880d681SAndroid Build Coastguard Worker individual tests). 43*9880d681SAndroid Build Coastguard Worker 44*9880d681SAndroid Build Coastguard Worker2. Consider move to identifying all tests by path-to-test-suite and then path to 45*9880d681SAndroid Build Coastguard Worker subtest, and don't use test suite names. 46*9880d681SAndroid Build Coastguard Worker 47*9880d681SAndroid Build Coastguard Worker Currently the test suite name is presented as part of test names, but it has 48*9880d681SAndroid Build Coastguard Worker no other useful function, and it is something that has to be skipped over to 49*9880d681SAndroid Build Coastguard Worker cut-and-paste a name to subsequently use to rerun a test. If we just 50*9880d681SAndroid Build Coastguard Worker represented each test suite by the path to its suite, then it would allow more 51*9880d681SAndroid Build Coastguard Worker easy cut-and-paste of the test output lines. This has the downside that the 52*9880d681SAndroid Build Coastguard Worker lines might get rather long. 53*9880d681SAndroid Build Coastguard Worker 54*9880d681SAndroid Build Coastguard Worker3. Allow 'lit' driver to cooperate with test formats and suites to add options 55*9880d681SAndroid Build Coastguard Worker (or at least sanitize accepted params). 56*9880d681SAndroid Build Coastguard Worker 57*9880d681SAndroid Build Coastguard Worker We have started to use the --params method more and more extensively, and it is 58*9880d681SAndroid Build Coastguard Worker cumbersome and error prone. Additionally, there are currently various options 59*9880d681SAndroid Build Coastguard Worker ``lit`` honors that should more correctly be specified as belonging to the 60*9880d681SAndroid Build Coastguard Worker ShTest test format. 61*9880d681SAndroid Build Coastguard Worker 62*9880d681SAndroid Build Coastguard Worker It would be really nice if we could allow test formats and test suites to add 63*9880d681SAndroid Build Coastguard Worker their own options to be parsed. The difficulty here, of course, is that we 64*9880d681SAndroid Build Coastguard Worker don't know what test formats or test suites are in use until we have parsed the 65*9880d681SAndroid Build Coastguard Worker input specifiers. For test formats we could ostensibly require all the possible 66*9880d681SAndroid Build Coastguard Worker formats to be registered in order to have options, but for test suites we would 67*9880d681SAndroid Build Coastguard Worker certainly have to load the suite before we can query it for what options it 68*9880d681SAndroid Build Coastguard Worker understands. 69*9880d681SAndroid Build Coastguard Worker 70*9880d681SAndroid Build Coastguard Worker That leaves us with the following options: 71*9880d681SAndroid Build Coastguard Worker 72*9880d681SAndroid Build Coastguard Worker * Currently we could almost get away with parsing the input specifiers without 73*9880d681SAndroid Build Coastguard Worker having done option parsing first (the exception is ``--config-prefix``) but 74*9880d681SAndroid Build Coastguard Worker that isn't a very extensible design. 75*9880d681SAndroid Build Coastguard Worker 76*9880d681SAndroid Build Coastguard Worker * We could make a distinction in the command line syntax for test format and 77*9880d681SAndroid Build Coastguard Worker test suite options. For example, we could require something like:: 78*9880d681SAndroid Build Coastguard Worker 79*9880d681SAndroid Build Coastguard Worker lit -j 1 -sv input-specifier -- --some-format-option 80*9880d681SAndroid Build Coastguard Worker 81*9880d681SAndroid Build Coastguard Worker which would be relatively easy to implement with optparser (I think). 82*9880d681SAndroid Build Coastguard Worker 83*9880d681SAndroid Build Coastguard Worker * We could allow fully interspersed arguments by first extracting the options 84*9880d681SAndroid Build Coastguard Worker lit knows about and parsing them, then dispatching the remainder to the 85*9880d681SAndroid Build Coastguard Worker formats. This seems the most convenient for users, who are unlikely to care 86*9880d681SAndroid Build Coastguard Worker about (or even be aware of) the distinction between the generic lit 87*9880d681SAndroid Build Coastguard Worker infrastructure and format or suite specific options. 88*9880d681SAndroid Build Coastguard Worker 89*9880d681SAndroid Build Coastguard Worker4. Eliminate duplicate execution models for ShTest tests. 90*9880d681SAndroid Build Coastguard Worker 91*9880d681SAndroid Build Coastguard Worker Currently, the ShTest format uses tests written with shell-script like syntax, 92*9880d681SAndroid Build Coastguard Worker and executes them in one of two ways. The first way is by converting them into 93*9880d681SAndroid Build Coastguard Worker a bash script and literally executing externally them using bash. The second 94*9880d681SAndroid Build Coastguard Worker way is through the use of an internal shell parser and shell execution code 95*9880d681SAndroid Build Coastguard Worker (built on the subprocess module). The external execution mode is used on most 96*9880d681SAndroid Build Coastguard Worker Unix systems that have bash, the internal execution mode is used on Windows. 97*9880d681SAndroid Build Coastguard Worker 98*9880d681SAndroid Build Coastguard Worker Having two ways to do the same thing is error prone and leads to unnecessary 99*9880d681SAndroid Build Coastguard Worker complexity in the testing environment. Additionally, because the mode that 100*9880d681SAndroid Build Coastguard Worker converts scripts to bash doesn't try and validate the syntax, it is possible 101*9880d681SAndroid Build Coastguard Worker to write tests that use bash shell features unsupported by the internal 102*9880d681SAndroid Build Coastguard Worker shell. Such tests won't work on Windows but this may not be obvious to the 103*9880d681SAndroid Build Coastguard Worker developer writing the test. 104*9880d681SAndroid Build Coastguard Worker 105*9880d681SAndroid Build Coastguard Worker Another limitation is that when executing the scripts externally, the ShTest 106*9880d681SAndroid Build Coastguard Worker format has no idea which commands fail, or what output comes from which 107*9880d681SAndroid Build Coastguard Worker commands, so this limits how convenient the output of ShTest failures can be 108*9880d681SAndroid Build Coastguard Worker and limits other features (for example, knowing what temporary files were 109*9880d681SAndroid Build Coastguard Worker written). 110*9880d681SAndroid Build Coastguard Worker 111*9880d681SAndroid Build Coastguard Worker We should eliminate having two ways of executing the same tests to reduce 112*9880d681SAndroid Build Coastguard Worker platform differences and make it easier to develop new features in the ShTest 113*9880d681SAndroid Build Coastguard Worker module. This is currently blocked on: 114*9880d681SAndroid Build Coastguard Worker 115*9880d681SAndroid Build Coastguard Worker * The external execution mode is faster in some situations, because it avoids 116*9880d681SAndroid Build Coastguard Worker being bottlenecked on the GIL. This can hopefully be obviated simply by 117*9880d681SAndroid Build Coastguard Worker using --use-processes. 118*9880d681SAndroid Build Coastguard Worker 119*9880d681SAndroid Build Coastguard Worker * Some tests in LLVM/Clang are explicitly disabled with the internal shell 120*9880d681SAndroid Build Coastguard Worker (because they use features specific to bash). We would need to rewrite these 121*9880d681SAndroid Build Coastguard Worker tests, or add additional features to the internal shell handling to allow 122*9880d681SAndroid Build Coastguard Worker them to pass. 123*9880d681SAndroid Build Coastguard Worker 124*9880d681SAndroid Build Coastguard Worker5. Consider changing core to support setup vs. execute distinction. 125*9880d681SAndroid Build Coastguard Worker 126*9880d681SAndroid Build Coastguard Worker Many of the existing test formats are cleanly divided into two phases, once 127*9880d681SAndroid Build Coastguard Worker parses the test format and extracts XFAIL and REQUIRES information, etc., and 128*9880d681SAndroid Build Coastguard Worker the other code actually executes the test. 129*9880d681SAndroid Build Coastguard Worker 130*9880d681SAndroid Build Coastguard Worker We could make this distinction part of the core infrastructure and that would 131*9880d681SAndroid Build Coastguard Worker enable a couple things: 132*9880d681SAndroid Build Coastguard Worker 133*9880d681SAndroid Build Coastguard Worker * The REQUIREs handling could be lifted to the core, which is nice. 134*9880d681SAndroid Build Coastguard Worker 135*9880d681SAndroid Build Coastguard Worker * This would provide a clear place to insert subtest support, because the 136*9880d681SAndroid Build Coastguard Worker setup phase could be responsible for providing subtests back to the 137*9880d681SAndroid Build Coastguard Worker core. That would provide part of the infrastructure to parallelize them, for 138*9880d681SAndroid Build Coastguard Worker example, and would probably interact well with other possible features like 139*9880d681SAndroid Build Coastguard Worker parameterized tests. 140*9880d681SAndroid Build Coastguard Worker 141*9880d681SAndroid Build Coastguard Worker * This affords a clean implementation of --no-execute. 142*9880d681SAndroid Build Coastguard Worker 143*9880d681SAndroid Build Coastguard Worker * One possible downside could be for test formats that cannot determine their 144*9880d681SAndroid Build Coastguard Worker subtests without having executed the test. Supporting such formats would 145*9880d681SAndroid Build Coastguard Worker either force the test to actually be executed in the setup stage (which 146*9880d681SAndroid Build Coastguard Worker might be ok, as long as the API was explicitly phrased to support that), or 147*9880d681SAndroid Build Coastguard Worker would mean we are forced into supporting subtests as return values from the 148*9880d681SAndroid Build Coastguard Worker execute phase. 149*9880d681SAndroid Build Coastguard Worker 150*9880d681SAndroid Build Coastguard Worker Any format can just keep all of its code in execute, presumably, so the only 151*9880d681SAndroid Build Coastguard Worker cost of implementing this is its impact on the API and futures changes. 152*9880d681SAndroid Build Coastguard Worker 153*9880d681SAndroid Build Coastguard Worker 154*9880d681SAndroid Build Coastguard WorkerMiscellaneous 155*9880d681SAndroid Build Coastguard Worker============= 156*9880d681SAndroid Build Coastguard Worker 157*9880d681SAndroid Build Coastguard Worker* Move temp directory name into local test config. 158*9880d681SAndroid Build Coastguard Worker 159*9880d681SAndroid Build Coastguard Worker* Support valgrind in all configs, and LLVM style valgrind. 160*9880d681SAndroid Build Coastguard Worker 161*9880d681SAndroid Build Coastguard Worker* Support ulimit. 162*9880d681SAndroid Build Coastguard Worker 163*9880d681SAndroid Build Coastguard Worker* Create an explicit test suite object (instead of using the top-level 164*9880d681SAndroid Build Coastguard Worker TestingConfig object). 165*9880d681SAndroid Build Coastguard Worker 166*9880d681SAndroid Build Coastguard Worker* Introduce a wrapper class that has a ``subprocess.Popen`` like interface 167*9880d681SAndroid Build Coastguard Worker but also supports killing the process and all its children and use this for 168*9880d681SAndroid Build Coastguard Worker running tests. This would allow us to implement platform specific methods 169*9880d681SAndroid Build Coastguard Worker for killing a process's children which is needed for a per test timeout. On 170*9880d681SAndroid Build Coastguard Worker POSIX platforms we can use process groups and on Windows we can probably use 171*9880d681SAndroid Build Coastguard Worker job objects. This would not only allow us to remove the dependency on the 172*9880d681SAndroid Build Coastguard Worker ``psutil`` module but would also be more reliable as the 173*9880d681SAndroid Build Coastguard Worker ``lit.util.killProcessAndChildren()`` function which is currently used is 174*9880d681SAndroid Build Coastguard Worker potentially racey (e.g. it might not kill a fork bomb completely). 175