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

Jump to bottom

chore: make it possible to debug integration tests with sbt or IDE #2327

Merged
merged 8 commits into from Dec 9, 2022
Merged

chore: make it possible to debug integration tests with sbt or IDE #2327

merged 8 commits into from Dec 9, 2022

Conversation

seakayone
Copy link
Collaborator

@seakayone seakayone commented Dec 7, 2022 •
edited

!-- Important! Please follow the guidelines for naming Pull Requests: https://docs.dasch.swiss/latest/developers/dsp/contribution/ -->

Pull Request Checklist

Task Description/Number

Make integration-tests pickup its own conf
Remove -Dconfig.resource=fuseki.conf param from IntegrationTest sbt config and replace by a src/it/resources/application.conf.

The it application.conf will be merged with the default one.

Remove import of standard application.conf in test.conf as this lead to a cyclic dependency during startup.

Add sbt debug javaOptions fix other commented out options

Show absolutePath in error message TriplestoreServiceHttpConnectorImpl

Documentation

This way is it possible (after building the sipi docker image locally) to start and debug the integration tests from the IntelliJ IDE without depending on sbt.

Basic Requirements

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix: represents bug fixes
  • Refactor: represents production code refactoring
  • Feature: represents a new feature
  • Documentation: documentation changes (no production code change)
  • Chore: maintenance tasks (no production code change)
  • Style: styles updates (no production code change)
  • Test: all about tests: adding, refactoring tests (no production code change)
  • Other... Please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Does this PR change client-test-data?

  • Yes (don't forget to update the JS-LIB team about the change)
  • No

@seakayone
Make integration-tests pickup its own conf from application.conf
ba6af95 
Remove -Dconfig.resource=fuseki.conf param from it sbt invocation and replace by a dedicated application.conf in it class pass.

The it application.conf will be merged with the default one.

Remove import of standard application.conf in test.conf as this lead to a cyclic dependency during startup.
@seakayone
Add sbt debug javaOptions fix other commented out options
9a3b118
@seakayone
Show absolutePath in error message
c3de0d8
@seakayone
Documentation
4a0c252
@seakayone seakayone self-assigned this Dec 7, 2022
@seakayone seakayone changed the title Wip/run it without sbt debugging chore: allow to run integrating-test in debugging Dec 7, 2022
@codecov
Copy link

codecov bot commented Dec 7, 2022 •
edited

Codecov Report

Base: 86.64% // Head: 5.24% // Decreases project coverage by -81.40% ⚠️

Coverage data is based on head (f29a867) compared to base (cedb603).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #2327       +/-   ##
==========================================
- Coverage   86.64%   5.24%   -81.41%     
==========================================
  Files         251     251               
  Lines       28266   28263        -3     
==========================================
- Hits        24491    1481    -23010     
- Misses       3775   26782    +23007
Impacted Files Coverage Δ
...org/knora/webapi/routing/admin/StoreRouteADM.scala 0.00% <0.00%> (-84.62%) ⬇️
...ore/impl/TriplestoreServiceHttpConnectorImpl.scala 0.00% <0.00%> (-77.06%) ⬇️
...bapi/src/main/scala/org/knora/webapi/package.scala 0.00% <0.00%> (-100.00%) ⬇️
...i/src/main/scala/org/knora/webapi/core/State.scala 0.00% <0.00%> (-100.00%) ⬇️
...rc/main/scala/org/knora/webapi/LanguageCodes.scala 0.00% <0.00%> (-100.00%) ⬇️
...c/main/scala/org/knora/webapi/OntologySchema.scala 0.00% <0.00%> (-100.00%) ⬇️
...c/main/scala/org/knora/webapi/core/AppRouter.scala 0.00% <0.00%> (-100.00%) ⬇️
...c/main/scala/org/knora/webapi/util/LogAspect.scala 0.00% <0.00%> (-100.00%) ⬇️
.../main/scala/org/knora/webapi/auth/JWTService.scala 0.00% <0.00%> (-100.00%) ⬇️
.../main/scala/org/knora/webapi/core/HttpServer.scala 0.00% <0.00%> (-100.00%) ⬇️
... and 196 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@seakayone seakayone changed the title chore: allow to run integrating-test in debugging chore: allow to debug integration-test with sbt or IDE Dec 7, 2022
@seakayone seakayone changed the title chore: allow to debug integration-test with sbt or IDE chore: make it possible to debug integration-test with sbt or IDE Dec 7, 2022
@seakayone seakayone changed the title chore: make it possible to debug integration-test with sbt or IDE chore: make it possible to debug integration-test with sbt or IDE Dec 7, 2022
@seakayone seakayone changed the title chore: make it possible to debug integration-test with sbt or IDE chore: make it possible to debug integration tests with sbt or IDE Dec 7, 2022
@seakayone seakayone marked this pull request as ready for review December 7, 2022 12:22
BalduinLandolt
BalduinLandolt approved these changes Dec 7, 2022
View reviewed changes
Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Choose a reason for hiding this comment

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

just one small point of criticism, overall it looks good

build.sbt
Comment on lines -203 to +204
// Test / javaOptions ++= Seq("-Dakka.log-config-on-start=on"), // prints out akka config
// Test / javaOptions ++= Seq("-Dconfig.trace=loads"), // prints out config locations
// IntegrationTest / javaOptions ++= Seq("-Dakka.log-config-on-start=on"), // prints out akka config
// IntegrationTest / javaOptions ++= Seq("-Dconfig.trace=loads"), // prints out config locations
// IntegrationTest / javaOptions += "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005", // starts sbt with debug port
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find those commented-out lines a bit irritating. If they have to stay there, please add a comment why/how/what for they are needed, otherwise I'd suggest removing them

Copy link
Collaborator Author

@seakayone seakayone Dec 9, 2022 •
edited

Choose a reason for hiding this comment

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

In general I agree that commented out code should be removed. Those Java options are documented and seemed useful for me in case we would like to debug the integration tests so I kept them.

irinaschubert
irinaschubert approved these changes Dec 7, 2022
View reviewed changes
Copy link

@irinaschubert irinaschubert left a comment

Choose a reason for hiding this comment

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

LGTM, although I don't think I fully understood what the problem was that you solved here.

docs/05-internals/development/testing.md Outdated Show resolved Hide resolved
docs/05-internals/development/testing.md Outdated Show resolved Hide resolved
seakayone and others added 3 commits December 7, 2022 14:36
@seakayone
Update docs/05-internals/development/testing.md
98c41ff 
Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
@seakayone
Update docs/05-internals/development/testing.md
73ba980 
Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
@seakayone
Merge branch 'main' into wip/run-it-without-sbt-debugging
a016c4b
@seakayone
Copy link
Collaborator Author

seakayone commented Dec 9, 2022 •
edited

LGTM, although I don't think I fully understood what the problem was that you solved here.

Before this change the configuration for fuskei was passed as JVM system property in the build.sbt. This is done by sbt when running the tests using sbt. However, this prevented my from starting the tests in my IDE since it starts the tests without sbt and the settings were not picked up. By renaming the fuseki.conf file to application.conf the application now picks it up automatically and merges the confs from both source sets main and it.

@seakayone seakayone merged commit 3a222bb into main Dec 9, 2022
@seakayone seakayone deleted the wip/run-it-without-sbt-debugging branch December 9, 2022 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irinaschubert irinaschubert irinaschubert approved these changes

@mpro7 mpro7 mpro7 approved these changes

@BalduinLandolt BalduinLandolt BalduinLandolt approved these changes

Assignees

@seakayone seakayone

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@seakayone @irinaschubert @mpro7 @BalduinLandolt