Tests Cleanup #245

l-kent · 2024-09-16T02:49:18Z

Removes many redundant tests, including all the no_plt_no_pic variants, which reduces the execution time by nearly half
Removes most of examples folder, which was largely very out of date, redundant, and not a good target for tests
Separated tests containing unresolved indirect calls into a new IndirectCallTests suite that replaces the existing IndirectCallsTests, containing many more tests and more detailed checks
Consistently uses BigInts for addresses since they can be longer than 32 bits (it may make sense to make them Longs instead)
Removed deprecated lifting scripts and replaces them with a single script, scripts/lift.sh which can be used to lift single examples
Removes deprecated MemoryRegionAnalysisMiscTest
Takes steps towards unifying test suite functionality with BASILTest (replacing TestUtil) now the base for many other tests
Adds new test suites AnalysisSystemTestsBAP and AnalysisSystemTestsGTIRB which just run all the tests in /correct and /incorrect with the static analysis enabled - this is very useful for finding bugs
Minor cleanup elsewhere in the tests

I'm still not sure what to do with all the histogram stuff in SystemTests - it doesn't really feel like it all belongs there, but I'm not sure where to put it.

I probably should move the cases where GTIRB successfully pre-resolves indirect calls back into the /correct folder, because they're at least tests of the lifter?

I'm not sure what to do with correct/syscall/clang_O2 - maybe it should be in IndirectCallTests? It regularly breaks things in interesting ways though and the indirect call isn't really the point?

…is enabled

…github

…nto their own folder

# Conflicts: # src/main/scala/ir/Program.scala # src/main/scala/translating/GTIRBToIR.scala # src/main/scala/translating/ReadELFLoader.scala # src/main/scala/util/RunUtils.scala # src/test/scala/IndirectCallsTests.scala # src/test/scala/IrreducibleLoop.scala # src/test/scala/LiveVarsAnalysisTests.scala # src/test/scala/MemoryRegionAnalysisMiscTest.scala # src/test/scala/PointsToTest.scala # src/test/scala/TaintAnalysisTests.scala # src/test/scala/ir/IRTest.scala

ailrst

SystemTests/arrays_simple and syscall/clang_O2 are still failing in system-tests, I suggest we add a suite for TDD style tests, functionally the same as system tests but just containing the borderline test cases.
Otherwise I have no strong objections.

src/test/scala/test_util/BASILTest.scala

src/test/scala/SystemTests.scala

ailrst · 2024-09-16T03:44:10Z

src/test/scala/SystemTests.scala

@@ -36,48 +26,52 @@ trait SystemTests extends AnyFunSuite {

  val testResults: mutable.ArrayBuffer[(String, TestResult)] = mutable.ArrayBuffer()

-  def runTests(programs: Array[String], path: String, name: String, conf: TestConfig): Unit = {
+  private val testPath = "./src/test/"
+


Can we set Logger.setLevel(LogLevel.WARN), printing the globals, proc addresses and then the same list of translation stage log messages for every test really isn't useful and makes it harder to see the failing/passing tests.

I just replaced most of the Logger.info calls with Logger.debug, only keeping ones to print the very basic test progress.

ailrst · 2024-09-16T03:49:15Z

.github/workflows/run-examples.yml

@@ -45,7 +42,7 @@ jobs:
        uses: actions/checkout@v4

      - name: System Tests
-        run: ./mill test.testOnly '*SystemTests*' || true
+        run: ./mill test.testOnly 'SystemTests*' || true


Now we expect SystemTests to pass we should remove the || true, its possibly also worth adding the AnalysisSystemTests as a separate stage.

We don't expect SystemTests to pass yet - arrays_simple doesn't pass yet for reasons unrelated to indirect calls and that has long been the case.

AnalysisSystemTests is not really worth putting in the GitHub action - it's useful to check for crashes but rather redundant in total. Many of the other test suites may be worth including though.

src/test/scala/test_util/BASILTest.scala

l-kent · 2024-09-16T05:24:24Z

Which are the borderline cases you mean? arrays_simple/gcc because it still fails? Getting it to pass requires a more sophisticated specification system.

ailrst · 2024-09-16T06:00:42Z

Which are the borderline cases you mean? arrays_simple/gcc because it still fails? Getting it to pass requires a more sophisticated specification system.

Yeah, if it requires new features to pass we should put it in a separate test suite to test new features against.

… suite

l-kent · 2024-09-17T04:14:18Z

Other tests that don't always pass right now are some of the malloc_memcpy_strlen_memset_free tests which tend to cause Z3 to time out (though I think this is slightly inconsistent). Should they go in their own suite, or in the same one as arrays_simple?

ailrst · 2024-09-17T04:24:34Z

I think we could relax the time limit (by 5 - 10 s?) so they pass reliably, since we don't have so many tests timing out now. They should consistently pass in a reasonable amount of time, clang is 8s on my laptop and gcc < 1s.

l-kent · 2024-09-17T04:40:19Z

Relaxing the time limit doesn't help for incorrect/malloc_memcpy_strlen_memset_free/gcc_O2:BAP but it does for correct/malloc_memcpy_strlen_memset_free/clang_O2:BAP.

…t tests, move syscall/clang_O2 to /indirect_calls

l-kent · 2024-09-17T23:29:55Z

correct/malloc_memcpy_strlen_memset_free/clang_O2:BAP still times out for the github action too. Maybe we should just put those tests in their own suite?

ailrst · 2024-09-17T23:43:29Z

Yeah that's probably best

…SpecTests with extended timeout), have github action expect SystemTests to pass

l-kent · 2024-09-18T00:54:05Z

I think this is good now. I've moved the malloc_memcpy_strlen_memset_free tests to their own ExtraSpecTests suite

l-kent added 24 commits September 5, 2024 13:12

remove redundant/uninteresting tests

6635d9d

remove no-plt_no_pic lifting option, remove deprecated lifting scripts

9b4efb6

close histogram csv, formatting, add options to run tests with analys…

e071190

…is enabled

remove redundant github action, ensure analysis tests are not run by …

122d977

…github

tidy irreducible loop test

64dcf6a

add indirect_call_outparam example as test

1804328

reorganise test structure, move tests need indirect call resolution i…

6503ba1

…nto their own folder

add markdown table and histogram files to gitignore

1731b7f

use BigInt for addresses since they can be longer than 32 bits

e2f6de3

delete deprecated mra test

1a067f3

remove most of examples folder - unnecessary and causes confusion

63895e9

improve lifting script + remove all other deprecated lifting scripts

b725563

clean up addresses being BigInts

0d9b291

update test syntax in readme

46e74f5

improved tests for indirect calls

f945165

remove interpreter tests that rely on examples folder

00dc468

work towards unifying test framework, general tests cleanup

6b4829b

fix CILVisitorTest

16fd70e

fix IndirectCallTests

9cf867c

clean up LiveVarsAnalysisTests

39c86f7

properly remove deprecated file

e99f925

minor fix

699a298

remove unused dotExpected files

9226f3b

l-kent requested a review from ailrst September 16, 2024 02:49

ailrst requested changes Sep 16, 2024

View reviewed changes

change most Logger.info calls to Logger.debug

ca0a60e

move AnyFunSuite functionality outside of BASILTest

cd19f49

l-kent added 2 commits September 17, 2024 13:31

move tests requiring unimplemented functionality to pass to their own…

3525b92

… suite

only run tests that exist

4e29d84

l-kent closed this Sep 17, 2024

l-kent reopened this Sep 17, 2024

l-kent added 2 commits September 17, 2024 15:54

increase time limit slightly

e4d4d31

move indirect call test cases with calls resolved by gtirb to /correc…

ef8ee53

…t tests, move syscall/clang_O2 to /indirect_calls

l-kent added 4 commits September 18, 2024 09:59

move histogram stuff to its own file

e56c5f7

improve use of TestResult

b607a37

move malloc_memcpy_strlen_memset tests to their own test suite (Extra…

6d49c85

…SpecTests with extended timeout), have github action expect SystemTests to pass

update GTIRB expected which were out of date

acd0377

ailrst approved these changes Sep 18, 2024

View reviewed changes

l-kent merged commit 3030d6b into main Sep 18, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests Cleanup #245

Tests Cleanup #245

l-kent commented Sep 16, 2024

ailrst left a comment

ailrst Sep 16, 2024

l-kent Sep 16, 2024

ailrst Sep 16, 2024

l-kent Sep 16, 2024

l-kent commented Sep 16, 2024

ailrst commented Sep 16, 2024

l-kent commented Sep 17, 2024

ailrst commented Sep 17, 2024

l-kent commented Sep 17, 2024

l-kent commented Sep 17, 2024

ailrst commented Sep 17, 2024

l-kent commented Sep 18, 2024

Tests Cleanup #245

Tests Cleanup #245

Conversation

l-kent commented Sep 16, 2024

ailrst left a comment

Choose a reason for hiding this comment

ailrst Sep 16, 2024

Choose a reason for hiding this comment

l-kent Sep 16, 2024

Choose a reason for hiding this comment

ailrst Sep 16, 2024

Choose a reason for hiding this comment

l-kent Sep 16, 2024

Choose a reason for hiding this comment

l-kent commented Sep 16, 2024

ailrst commented Sep 16, 2024

l-kent commented Sep 17, 2024

ailrst commented Sep 17, 2024

l-kent commented Sep 17, 2024

l-kent commented Sep 17, 2024

ailrst commented Sep 17, 2024

l-kent commented Sep 18, 2024