Ticket #10150 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Slow Unit Tests - LoadTest

Reported by: Owen Arnold Owned by: Martyn Gigg
Priority: major Milestone: Release 3.3
Component: Framework Keywords: Maintenance
Cc: Blocked By:
Blocking: #9905 Tester: Owen Arnold

Description (last modified by Owen Arnold) (diff)

We have a number of slow running unit tests which we need to fix as part of the 3.3 maintenance window. We are targeting test suits than execute in > 2 seconds on our clean rhel6 build servers. For IO tests (Loading & Saving) we have applied a more generous threshold of > 5 seconds.

See the full list to see which tests have been assigned to you: http://www.mantidproject.org/images/2/2f/Slow_tests.xlsx_-_Sheet1.pdf

Criteria

  1. Test suits (each test class instance) should execute in < 1 second as a rough target
  2. As a corollary to the above, If the test suite contains lots of test methods aim for < 0.1 second per test method
  3. If for some reason you get the speed up the test below the target using the strategies listed below, you need to justify why when you close the ticket.

Guidelines/instructions to help

  1. Keep tests readable and improve code readability where possible. Unit tests are documentation.
  2. Do not delete test methods without good reason. We do not want to reduce test coverage
  3. Changing the algorithm code to improve speed is OK, but exercise caution. Add additional test coverage if it's not already good enough.
  4. Take test methods that are slow and involve IO, or are processor intensive and make them into system tests. Integration tests are not Unit tests.
  5. Most of the speed improvements will probably come from better selection of input data. Caching input data is also a good option.
  6. Create sub tickets for algorithms or groups of algorithms to help testability if you wish, but mark this ticket as the 'blocked' by each one.

Roman was originally assigned this, but doesn't know how to speed it up. So I'm assigning this to the original author.

Change History

comment:1 Changed 6 years ago by Owen Arnold

  • Description modified (diff)
  • Summary changed from Slow Unit Tests for Roman Tolchenov (cloned) to Slow Unit Tests - LoadTest

comment:2 Changed 6 years ago by Owen Arnold

  • Tester Owen Arnold deleted

comment:3 Changed 6 years ago by Owen Arnold

  • Owner changed from Roman Tolchenov to Martyn Gigg
  • Status changed from new to assigned

comment:4 Changed 6 years ago by Martyn Gigg

  • Status changed from assigned to inprogress

Remove tests for specific file types.

These are covered by the more LoadLotsOfFiles system test and offer no additional benefit. Refs #10150

Changeset: 2105aa687cc03ce1399c3e4e3adb5c0197de3693

comment:5 Changed 6 years ago by Martyn Gigg

Don't execute a load just test what would Loader was chosen.

Refs #10150

Changeset: 628e187a0ea8d5beae1c7f8e4b50579278f0cd83

comment:6 Changed 6 years ago by Martyn Gigg

Remove a lot of Load executions in LoadTest

Instead just check what files it would be expected to Load. A new system test, LoadTest.py, will be created to do the more exhaustive checking that loading the files requires. Refs #10150

Changeset: 1897bf740cf508e523ba7a985127671ddda68d80

comment:7 Changed 6 years ago by Martyn Gigg

Add in a set of .expected files for additional Load checks

Refs #10150

Changeset: 492d8cca0ccf83c2c1db42f030e67e925ff3bfb7

comment:8 Changed 6 years ago by Martyn Gigg

Add some more test files for Load along with .expected files.

Refs #10150

Changeset: 95879678a796a05fb48d7325d3f079b33d0f65c4

comment:9 Changed 6 years ago by Martyn Gigg

First cut at LoadTest that covers what the unit test was doing.

Refs #10150

Changeset: ab10634d6c63705f7228433b81e3f33e0ccbde63

comment:10 Changed 6 years ago by Martyn Gigg

Extend LoadTest to cover the functionality that the unit test covered.

There are also extra checks to confirm that the multi-file loading behaviour is correct. Refs #10150

Changeset: 0288222c6a0fae40eceb001a686585410fa274aa

comment:11 Changed 6 years ago by Martyn Gigg

Use the correct variable name in the .expected files.

Refs #10150

Changeset: fc6fcd691e526dd3cdf68fd78d1ea7b2dd9371b6

comment:12 Changed 6 years ago by Martyn Gigg

Fix bug with .expected files

Refs #10150

Changeset: 73086266070b2da1df6a2b6c7da8c33389dced3b

comment:13 Changed 6 years ago by Martyn Gigg

  • Status changed from inprogress to verify
  • Resolution set to fixed

Branch: maint/10150_improve_loadtest_performance in BOTH mantid and systemtests repositories.

Tester: All unit test and system tests should be passing. The main speedup was gained from moving out the execution of the Load algorithm for different file types to a system test. The unit test is left checking such things as simply finding the correct files but not attempting to Load them.

comment:14 Changed 6 years ago by Owen Arnold

  • Status changed from verify to verifying
  • Tester set to Owen Arnold

comment:15 Changed 6 years ago by Owen Arnold

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/maint/10150_improve_loadtest_performance'

Full changeset: 3f5af09051fc96ebff306f43c2d4e324eef81644

comment:16 Changed 6 years ago by Owen Arnold

Code review looks good. New system tests passing. Unit tests passing. No overall lost of test functionality. Executes in 1 second now.

comment:17 Changed 6 years ago by Owen Arnold

Merge remote-tracking branch 'origin/maint/10150_improve_loadtest_performance'

Full changeset: 32b306aaeff51f893ad9bb54803fd886d11d8072

comment:18 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10992

Note: See TracTickets for help on using tickets.