chore: Added capability of running ITs on maven#38354
Conversation
WalkthroughThis pull request introduces modifications to the Appsmith server's Git-related components, focusing on artifact type management and test configuration. The changes involve updating the Changes
Sequence DiagramsequenceDiagram
participant GitContext
participant GitServerInitializerExtension
participant ArtifactService
GitContext->>GitContext: Store artifactType in contextStore
GitServerInitializerExtension->>ArtifactService: createOrUpdateSshKeyPair(artifactType, artifactId, keyType)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/server/pom.xml (1)
80-103: Adding multiple test sources via build-helper-maven-plugin
Definingsrc/test/java,src/test/it, andsrc/test/utilsallows a cleaner test organization. This step helps maintain clarity between unit, integration, and utility tests.Consider adding comments or documentation explaining the semantic purpose of each test directory.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/server/appsmith-server/src/test/it/com/appsmith/server/git/templates/contexts/GitContext.java(1 hunks)app/server/appsmith-server/src/test/utils/com/appsmith/server/git/GitServerInitializerExtension.java(3 hunks)app/server/pom.xml(3 hunks)
🔇 Additional comments (8)
app/server/appsmith-server/src/test/it/com/appsmith/server/git/templates/contexts/GitContext.java (1)
28-28: Storing artifactType in the context store
This additional context store entry helps streamline the retrieval of artifactType in the extension. The change is minimal and makes sense in ensuring consistent artifact-type handling downstream.
app/server/appsmith-server/src/test/utils/com/appsmith/server/git/GitServerInitializerExtension.java (3)
4-5: Replacing ApplicationService with ArtifactService
Switching to ArtifactService clearly aligns the implementation with artifact-centric operations. This change appears consistent with the new workflow.
Also applies to: 40-40
59-59: ArtifactType usage
Retrieving ArtifactType from the parent context ensures the appropriate artifact type is mapped before creating the SSH key pair. This is a good design approach.
67-67: Creating or updating SSH key pair
The createOrUpdateSshKeyPair call on artifactService is well-structured. Ensure the returned mono (i.e., gitAuthMono) handles errors gracefully in upstream code.
app/server/pom.xml (4)
46-48: Granular test skipping flags
Introducing <skipITs>, <skipTests>, and <skipUTs> provides fine control over test execution. This meets the PR objective of distinguishing UT from IT runs.
114-116: Surefire test configuration
Setting <testSourceDirectory> to src/test/java and using <skipTests>${skipUTs}</skipTests> is a direct and effective way to skip only unit tests when needed.
117-129: JUnit Jupiter engine dependency for unit tests
It’s good to see version consistency across JUnit dependencies. Excluding junit-platform-commons is consistent with your test framework strategy.
130-148: Failsafe integration test configuration
Pointing <testSourceDirectory> to src/test/it and referencing <skipITs> matches the intended separation of UTs and ITs. This streamlines build management.
|
|
||
| @Autowired | ||
| ApplicationService applicationService; | ||
| ArtifactService artifactService; |
There was a problem hiding this comment.
Shouldn't this be parameterized?
There was a problem hiding this comment.
probably not in the scope of this PR, however something to note down
2 participants
Description
Enables running UTs and ITs as desired on maven. The skipTests command continues to skip all tests.
To run only UTs, use:
mvn testTo run only ITs, use:
mvn verify -DskipUTs=trueAutomation
/ok-to-test tags="@tag.Git"
Summary by CodeRabbit
New Features
artifactTypewith theextensionContext.Bug Fixes
Documentation
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12483043596
Commit: 9b65433
Cypress dashboard.
Tags:
@tag.GitSpec:
Tue, 24 Dec 2024 15:24:20 UTC