Skip to content

Add architecture tests. - #1754

Open
fduminy wants to merge 5 commits into
assertj:3.xfrom
fduminy:architecture_test
Open

Add architecture tests.#1754
fduminy wants to merge 5 commits into
assertj:3.xfrom
fduminy:architecture_test

Conversation

@fduminy

@fduminy fduminy commented Jan 12, 2020

Copy link
Copy Markdown
Contributor

Check List:

  • Fixes nothing
  • Unit tests : NA
  • Javadoc with a code example (on API only) : NA

Remarks :

  • it's a first attempt to define layers, feel free to comment about current/wanted architecture
  • I have started to work on other rules (for another PR)

@fduminy

fduminy commented Jan 29, 2020

Copy link
Copy Markdown
Contributor Author

any feedback ?

@joel-costigliola

joel-costigliola commented Jan 29, 2020

Copy link
Copy Markdown
Member

I'll have a look at it this week, thansk @fduminy.

joel-costigliola
joel-costigliola reviewed Jan 30, 2020
View reviewed changes
Comment thread pom.xml Outdated
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
GROUPS_LAYER,
MATCHER_LAYER,
PRESENTATION_LAYER,
UTIL_LAYER);

@joel-costigliola joel-costigliola Jan 30, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to think about the dependencies rules of AssertJ (one is ideally util should not depend on internal but whether this is actually true or not is not sure at all).
I think a reasonable approach is to set the frame for architecture tests in this PR and add the rules AssertJ should follow in subsequent PR that would also refactor the code to enforce them.

@fduminy fduminy Feb 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the code to check the rule "util should not depend on internal" and the test pass.

@joel-costigliola joel-costigliola Feb 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks!

@fduminy fduminy Feb 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, due to bugs in the test, it didn't pass for the rule "util should not depend on internal"
but now it's "fixed" by freezing the violation in archunit_store/a7253140-a775-4d8b-8f63-e57ad212fdea

@fduminy

fduminy commented Feb 1, 2020

Copy link
Copy Markdown
Contributor Author

it seems there is temporary network failure from travis and If don't know how to restart the build manually.

@scordio

scordio commented Feb 1, 2020

Copy link
Copy Markdown
Member

it seems there is temporary network failure from travis and If don't know how to restart the build manually.

Closing and reopening the PR does the trick

@scordio scordio closed this Feb 1, 2020
@scordio scordio reopened this Feb 1, 2020
@scordio

scordio commented Feb 1, 2020

Copy link
Copy Markdown
Member

it seems there is temporary network failure from travis and If don't know how to restart the build manually.

Still the same, let's wait a bit more

joel-costigliola
joel-costigliola reviewed Feb 1, 2020
View reviewed changes
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
joel-costigliola
joel-costigliola reviewed Feb 1, 2020
View reviewed changes
Comment thread src/test/java/org/assertj/core/architecture/ArchitectureTest.java Outdated
@joel-costigliola

joel-costigliola commented Feb 1, 2020

Copy link
Copy Markdown
Member

@scordio the build problem is related to Travis I guess it's time to move all our builds to github actions ;-)

if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then ./mvnw clean verify javadoc:javadoc; fi
--2020-02-01 08:24:07--  https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar
Resolving repo.maven.apache.org (repo.maven.apache.org)... 151.101.200.215
Connecting to repo.maven.apache.org (repo.maven.apache.org)|151.101.200.215|:443... connected.
HTTP request sent, awaiting response... 403 Forbidden

@fduminy fduminy closed this Feb 1, 2020
@fduminy fduminy reopened this Feb 1, 2020
@fduminy

fduminy commented Feb 1, 2020

Copy link
Copy Markdown
Contributor Author

If you agree with this PR, there is now a kind of TODO in archunit_store/a7253140-a775-4d8b-8f63-e57ad212fdea

@joel-costigliola
joel-costigliola changed the base branch from master to main June 21, 2020 05:33
@joel-costigliola joel-costigliola added this to the 4.0.0 milestone Jul 26, 2022
@scordio
scordio force-pushed the main branch 2 times, most recently from 38c7b4b to 3cde5bf Compare July 20, 2023 15:20
@scordio
scordio force-pushed the 3.x branch 2 times, most recently from 301ca01 to c730d18 Compare June 1, 2024 16:04
@scordio scordio modified the milestones: 4.0.0-M1, 4.0.0-M2 Jan 31, 2025
@joel-costigliola joel-costigliola modified the milestones: 4.0.0-M2, 4.x Jul 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joel-costigliola joel-costigliola joel-costigliola left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.x

Development

Successfully merging this pull request may close these issues.

3 participants

@fduminy @joel-costigliola @scordio