V8-archive: 3/4 of the Unit Tests fail

[syslogic]

Bug Report

Steps to Reproduce

1. Refactor from PHPUnit_Framework_TestCase to \PHPUnit\Framework\TestCase
2. Then click on 'Tests' in the IDE.

Expected Behavior

The unit tests should be matching the source code.

Actual Behavior

The unit tests generally seem quite outdated and barely match the source code.

Other Context & Screenshots

This commit has the changes required to even run the unit tests:
https://github.com/syslogic/directus-api/commit/788f66e7e0a7b5a0628cd5294ac2f5948ae8e355

While the result looks about like this:

so far, I'm at 36.55% passed tests... the main issue still are classes & methods which do not exist anymore - and the lack of API documentation, which one could use as an ultimate reference for writing the tests, so that the source code could be properly validated against them.

Technical Details

• Device: [Desktop]
• OS: [Redhat Linux, Kernel 3.10]
• Web Server: [Apache 2.4.37]
• PHP Version: [7.1.26]
• PHPUnit Version [7.5.7]
• Database: [MariaDB 10.2.22]
• Install Method: [cloned the master branch]

[benhaynes]

Our previous API Lead had limited time and had to stop updating tests so that he had enough time to fix issues. Now that things are closer to being stable we should find time to go back and get all tests passing... and add _more_ test coverage.

[rijkvanzanten]

Also, we might want to consider writing input-output tests separately from the API implementation, that way we can also test future ports on the same tests

[syslogic]

There indeed seems to be some disconnect at some point of time (maybe 2-3 major version numbers ago), where only the source had been developed further, without the tests being updated accordingly, which subsequently lead to the current situation - which is a) barely testable and b) barely maintainable (no matter if the code works, whether or not). I can try to see how far I'd get with apigen, the problem is just that this would require inline documentation being present, from which it would generate the documentation (similar to JavaDocs). Top priority should be basic functionality and once this can be tested, one can convert issues from border-cases into test-cases. I've already added further unit tests skeletons with skelgen, which so far test nothing, but have have the correct file-names and method names. the coverage report currently tells me 10% (while this value might be inaccurate, due to a large number of tests not covering anything).

[syslogic]

@rijkvanzanten such tests already exist in the tests/io directory... where guzzlehttp most likely should be a development dependency, unless it is being used in the source-code, too.

[syslogic]

just seen this commit https://github.com/directus/api/commit/39baa80395b556ada02e49e63b770b248cc85925 when wanting the create a branch for phpunit7 - and there isn't the any sane reasoning provided in the commit message, even if the project depends on php 7.1, which renders the dependency for phpunit 5 completely pointless; it only reintroduces the previous version mismatch. this suggests, that painting the problem is being preferred, instead of engineering a proper solution. this attitude is exactly what lead to this situation. Sorry, but I see no use for me here; just keep on with the crap, which the previous developer produced. "no time" is a pretty lame excuse for not being able to program according to the standards... the amount of difficult to reproduce issues speaks for itself. technology > religion.

[benhaynes]

Hey @syslogic — thank you for the thoughtful responses. Though I think there's a slight miscommunication here:

Directus is used on some of our client projects, and around the middle of last year our developer only had 2 days/week to work on Directus. At the time, there were many issues that needed to be resolved for those paying clients which meant we had to take on this technical debt. It's not something I/we wanted to do, but it was an unfortunate requirement.

There are _very_ few free contributors. Therefore, I have to personally pay for all of our Directus developers to keep this (free and open source) project alive. When a project had zero revenue, it's just a real-world constraint that sometimes we need to skip tests to meet deadlines. We don't have a choice unless community developers want to donate their time for free or a company is willing to financially sponsor this development.

At this point my _only_ goals for Directus are: stability, docs, and (passing) test coverage. I have extremely limited resources, but would be happy to discuss paid contractor work to get our Directus API tests passing and increase coverage.

Is that something you would be interested in and capable of? Is there anyone else reading this that would like to join our core team and take on this role?

If not, then I hope it's at least easier to understand my position.

[rijkvanzanten]

@syslogic that change broke building the api for release, so i had to revert it in order to release the API

[benhaynes]

Since we are moving to Functional Tests and these Unit Tests are not working, should we remove this code to avoid confusion?

@rijkvanzanten @hemratna @bjgajjar 🔔

[hemratna]

Let's close this issue as we are moving to functional tests.
Are we going to delete the tests directory from API repo?

[benhaynes]

I agree we should close it — but I think we should remove these broken tests from the codebase first.

[rijkvanzanten]

🔥

[benhaynes]

OK, let's remove these tests and then we can close this as a Feature Request.

[hemratna]

Fixed in #1244