Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Architecture Tests #2617

Closed
lenhard opened this issue Mar 6, 2017 · 4 comments
Closed

Improve Architecture Tests #2617

lenhard opened this issue Mar 6, 2017 · 4 comments
Labels
status: help wanted type: code-quality Issues related to code or architecture decisions type: enhancement

Comments

@lenhard
Copy link
Member

lenhard commented Mar 6, 2017

I just noticed that our architecture tests currently do not cover static imports and, therefore, are missing at least one dependency that we want to forbid.

The class org.jabref.logic.pdf.PdfAnnotationImporter currently includes this statement import static org.jabref.gui.importer.actions.OpenDatabaseAction.LOGGER. Apart from being totally unnecessary (there would be no problem in using an own logger for the class), this is also forbidden. A fix to this issue should:

  1. remove the dependency in PdfAnnotationImporter and using an own logger.
  2. enhance the ArchitectureTests to detect static imports (which could lead to discovering further errors that might not be as easy to fix)
@simonharrer
Copy link
Contributor

Idea: there may be checkstyle rules that can be configured to cover our architecture tests.

@LinusDietz
Copy link
Member

Oh, that was me. Fixed this instance in #2618. Unfortunately, I don't have time to investigate 2.

However, I am happy to have raised attention to this problem by committing this mistake.

@tobiasdiez
Copy link
Member

Further architecture related desirable tests are:

  • logic and viewModel: no javafx except javafx.collections
  • view, viewModel and controller: no Globals and JabRefGUI, no direct FXDialogService
  • all fxml files can be loaded successfully
  • no fetcher is used directly outside of the import package

@tobiasdiez tobiasdiez added type: code-quality Issues related to code or architecture decisions and removed architecture labels Apr 1, 2017
@LinusDietz
Copy link
Member

There is a new tool called ArchUnit, maybe this is helpful. https://github.com/TNG/ArchUnit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted type: code-quality Issues related to code or architecture decisions type: enhancement
Projects
None yet
Development

No branches or pull requests

5 participants