Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

The functional tests implementation is in process.

Common Tools

There is a set of tools used by all the tests. These tools will be described in this section.

Maven

Structural Issues

During the development the following issues have been detected in the code, which make Unit Testing harder and impact the maintainability of the code:

* Hidden dependencies. Many objects require other objects (like DatabaseManager) but the dependency is never explicitly declared or set. These dependencies should be fulfilled as parameters in the constructors or factory methods.

* Hidden constructors. It would be advisable to have public constructors in the objects and provide a Factory class that manages the instantiation of all required entities. This would facilitate testing and provide a separation of concerns, as the inclusion of the factory methods inside objects usually adds hidden dependencies (see previous point).

Refactoring would be required to fix these issues.

Development Issues

During development some issues have arised. These issues need to be solved for the unit tests to be viable.

Warnings to be taken in account:
  • Unit tests may be faulty due to misunderstanding of the source code, a revision is required to ensure they behave as expected
  • Unit tests may be incomplete, missing some paths of the existing code
  • Due to the tight dependencies between some classes some methods can't be tested completely. In these cases more benefit can be obtained from integration tests than from unit tests
  • For the aforementioned reasons, a revision (peer-review) is required to ensure the unit tests behave as expected and they are reliable
  • The unit tests may lack tests for some edge cases. It's in the nature of Unit Tests to evolve as new bugs are found, so they should be added as they are detected (via peer-review or via bug reports)
Issues found:
  • A mock of BrowseCreateDAOOracle has been done due to an incompatibility between H2 and the "upper" function call. This will affect tests related to case sensitivity in indexes.
  • There is an issue with actions that delete an Item, the test gets a timeout trying to connect the database. It seems related to the mock mentioned in the previous point, but the source of the error has not been found. The table gets locked and raises exception SQLNestedException: Cannot get a connection, pool error: Timeout waiting for idle object
  • Many objects (like SupervisedItem) lack a proper definition of the "equals" method, which makes comparison between objects and unit testing harder
  • Update method of many objects doesn't provide any feedback, we can only test if it raises an exception or not, but we can't be 100% sure if it worked 
  • Many objects have methods to change the policies related to the object or children objects (like Item), it would be good to have some methods to retrieve these policies also in the same object (code coherence)
  • There are some inconsistencies in the calls to certain methods. For example getName returns an empty String in a Collection where the name is not set, but a null in an Item without name
  • DCDate: the tests raise many errors. I can't be sure if it's due to misunderstanding of the purpose of the methods or due to faulty implementation (probably the previous). In some cases extra encapsulation of the internals of the class would be advisable, to hide the complexities of the Calendars (months starting by 0, etc)
  • The Authorization system gets a bit confusing. We have AuthorizationManager, AuthorizationUtils, methods that raise exceptions and methods that return booleans. Given the number of checks we have to do for permissions, and that some classes call methods that require extra permissions not declared or visible at first, this makes creation of tests (and usage of the API) a bit complex. I know we can ignore all permissions via context (turning on and off authorizations) but usually we don't want that
  • Bitstream: there are no checks to ensure a bitstream has been deleted. The removal is logical (flag in database) so a method to verify that would be advisable (at least for the purpose of testing)
  • Bundle: it's raising an issue with the column named "name". The column is accessible in the database manually (out of the test) via SQL, and it's the only column having this issue, need to check in more detail why is this happening
  • Community: set logo checks for authorization, but set metadata doesn't. It's in purpose?
  • Community: when you create a subcommunity, it is not added as a child straight away
  • Collection: methods create and delete don't check for authorization
  • FormatIdentifier: there is an error when checking if the filename is null, as the method lowerCase is called before the check, raising NPE is the entry is null.
  • Item: there is no authorization check for changing policies, no need to be an administrator
  • ItemIterator: it uses ArrayLists in the methods instead of List
  • ItemIterator: we can't verify if the Iterator has been closed
  • LicenseUtils: the template systems seems faulty. When using the parameters, %0 and %1  return the same value
  • Site: this class has some empty methods, like delete or update
  • Site: this class extends DSpaceObject. With it being a Singleton, it creates potential problems, for example when we use DSpaceObject methods to store details in the object. It's this relation necessary?
  • SupervisedItem: seems to have some issue when finding items by EPerson
Proposals:

To solve the previous issues, some proposals are done:

  • Database dependency causes too many issues, making unit testing much harder and increasing the complexity of the code. Refactoring to a database-neutral system should be a priority

Common Tools

There is a set of tools used by all the tests. These tools will be described in this section.

Maven

The build tool for DSpace, Maven, will also be used to run the The build tool for DSpace, Maven, will also be used to run the tests. For this we will use the Surefire plugin, which allows us to launch automatically tests included in the "test" folder of the project. We also include the Surefire-reports plugin in case you are not using a Continous Integration environment that can read the output and generate the reports.

...

There is a base class called "AbstractUnitTest". This class contains a series of mocks and references which are necessary to run the tests in DSpace, like mocks of the DatabaseManager object. All Unit Tests should inherit this class, located under the package "org.dspace" in the test folder of DSpace-test.

About the implementation, several objects only offer a hidden constructor and a factory method to create an instance of the object. This means we effectively have to create them using available factory methods. Other specifics have been commented above, like:

  • Usage of a temporal file system
  • Usage of an in-memory database (h2)
  • Mocking the DatabaseManager class

Code Issues

During the development the following issues have been detected in the code, which make Unit Testing harder and impact the maintainability of the code:

* Hidden dependencies. Many objects require other objects (like DatabaseManager) but the dependency is never explicitly declared or set. These dependencies should be fulfilled as parameters in the constructors or factory methods.

* Hidden constructors. It would be advisable to have public constructors in the objects and provide a Factory class that manages the instantiation of all required entities. This would facilitate testing and provide a separation of concerns, as the inclusion of the factory methods inside objects usually adds hidden dependencies (see previous point).

...

"AbstractUnitTest". This class contains a series of mocks and references which are necessary to run the tests in DSpace, like mocks of the DatabaseManager object. All Unit Tests should inherit this class, located under the package "org.dspace" in the test folder of DSpace-test.

About the implementation, several objects only offer a hidden constructor and a factory method to create an instance of the object. This means we effectively have to create them using available factory methods. Other specifics have been commented above, like:

  • Usage of a temporal file system
  • Usage of an in-memory database (h2)
  • Mocking the DatabaseManager class

Integration Tests

These tests work at the API level and test the interaction of components within the system. Some examples, placing an item into a collection, or creating a new metadata schema and adding some fields. Primarily these tests operate at the API level ignoring the interface components above it.

...