Hi Petri, ... I'm totally with you on making sure that we don't introduce regressions with Testcontainers. I'm not too concerned about those 60 something failing tests at this point; I did this exercise already a while back and remember most of those; they fail, because files are not accessible inside the test container, so a simple volume mapping will fix the problems. Which leaves me with a few (10-15) that I still have to figure out, but usually it's either some kind of concurrency issue or the order of the tests being executed or both (very rarely some really exotic issue... don't really expect that). Now, the real challenge is execution time. +45min doesn't sound very appealing, but I can explain a bit more what options we have:
- the current configuration makes sure that we start a *single container* for the whole *test suite*; a couple of months ago I made a mistake when I took my first shot at this problem and had "reset the container for every single test (method)"; obviously that one is slow as molasses; not a problem anymore - I tried yesterday a very *aggressive setting* by allowing *parallel tests (methods) execution* which is in fact faster (should be faster than 1h), but unfortunately the *error rate is very high* with the current way the tests are written; I didn't investigate too much, because I wanted to keep the tests as much as possible as they are (given the recent effort to make them more stable); so for now, not an option (unless I find a magic trick... not sure) - ... but: there's still some kind of parallelism that we can allow (this is currently configured with the PR)... test methods are executed in sequence, but on a *class level* we can allow *parallel execution*; that helps a bit, but we are hovering around the 2h mark and at the moment; I am not sure at this point if I can lower this without significant changes This is what I have so far and the goal is still to get the time down to a comparable time frame (1h 30min would be already great). Just to clarify concerning running the two ways of testing in parallel for a while: once all is fixed here we don't need to merge immediately... we can leave that PR open for whatever time we see fit and until everyone feels comfortable with the results. I'll just rebase the PR with whatever changes arrive from upstream and let the rerun until we are happy; I can take care of that. NOTE: all this is in preparation for another proposal how we could test the same set of features like we do now, but a lot faster (like millis to seconds per test instead of minutes, aggressive parallel execution on a method level and no need for a container); but that's for a different day and probably multiple PRs (starting with a smaller proof of concept). There was an initial discussion with Istvan, Ed and others about it that needs to be continued (maybe you were there too?). Speed was not the only criteria in this discussion; there are a lot of people with knowledge about the business processes, but they lack coding skills... this new type of tests would allow us to engage them without requiring them to specify tests as code. These tests would also help with another question that I get quite frequently: "what's the test coverage of Fineract"?. The close to 900 integration tests we currently have should cover actually a broad range of Fineract's feature set, but we can't use tools (Jacocco) to pin a number on that. Just FYI. So... bringing down the time... that's the next thing to do... let's see. Thanks again for keeping an eye on the details, it's easy to miss something important, appreciate it. Cheers, Aleks On Wed, Jan 5, 2022 at 12:03 AM Petri Tuomola <[email protected]> wrote: > Hi Aleks - firstly, many thanks for your excellent work! The list of > improvements you’ve put forward is very impressive and will be a huge help > in speeding up and improving the developer experience for Fineract. Great > job! > > On this testing one specifically, I think we should be careful to ensure > the tests pass consistently before merging this. There’s been quite a lot > of work to try to reduce the “flakiness” of the existing tests by fixing > the ordering etc. I think overall we are in a much better position now than > a year ago - typically now the integration tests will pass without too much > pain. > > Perhaps the best way would be to have this running in parallel for a while > - i.e. incorporate the new way of running tests as a separate workflow > whilst keeping the existing one running as well? We could then compare the > failure rate to ensure they were equally stable before decommissioning the > existing workflow. > > I think the same applies for performance… I hope this will improve testing > performance, but currently (with the ‘old method’) the integration tests > typically complete in around 1 hour 15 minutes… so if this now takes up to > 2 hours, we should see if there are further ways to improve. > > Thanks again for all your great work! > > Cheers > Petri > > > > On 4 Jan 2022, at 17:35, Aleksandar Vidakovic <[email protected]> > wrote: > > > > Hi everyone, > > > > ... on Github Actions the tests ran just under 2 hours, but with 63 > failures (I removed for now the "--fail-fast" flag to get an idea how long > this takes overall). > > > > Some of the tests seem to depend implicitly on another one to provide > some data so that the other test needs to run first. Some tests other tests > do not seem to be thread safe. > > > > A big chunk of the current failures are quite trivial (all image, import > and document management tests). They fail, because they expect some local > files to be available (read: in the Docker container they run in), but > those files are extracted from the test sources outside the container... > not a big surprise that these files are obviously not visible to the > fineract instance, easy fix. > > > > And then finally the 2FA and OAuth2 tests are failing, because I thought > I could run them together with the other tests... when I execute them > separately then things work just fine... so probably I'll just move them > again to their current separate modules. > > > > A quick count shows that 44 of the currently failing tests are easy to > fix. Which brings the current success rate closer to 99%. > > > > After this work is done we should migrate those tests as fast as > possible to Cucumber/BDD tests that don't have the need for a full blown > Fineract instance. That type of testing should be much faster. > > > > Just FYI. > > > > Cheers, > > > > Aleks > >
