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.

Sign up for GitHub

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

provide authoritative list of desired integration tests #7897

Closed
donsizemore opened this issue May 25, 2021 · 3 comments · Fixed by #7915
Closed

provide authoritative list of desired integration tests #7897

donsizemore opened this issue May 25, 2021 · 3 comments · Fixed by #7915

Comments

@donsizemore
Copy link
Contributor

Currently, our automated testing references @pameyer's docker-aio work:
https://github.com/IQSS/dataverse/blob/develop/conf/docker-aio/run-test-suite.sh

Other testing implementations have been encouraged to use this script lest the Dataverse project have to maintain multiple lists of integration tests.

The run-test-suite.sh script includes the line
# TODO: Rather than hard-coding the list of "IT" classes here, add a profile to pom.xml.

However, when I added an all-integration-tests profile to pom.xml in a branch, including **/*.java runs more tests than are currently desired:

we’ve been excluding a few IT tests (by not enumerating them)

If IQSS prefers to enumerate integration tests, I propose a stand-alone, authoritative list of desired tests which run-test-suite.sh and other testing implementations could reference from the Dataverse repo, possibly under tests/

I also propose adding an all-integration-tests profile to pom.xml to complement the all-unit-tests profile, which would be disabled by default but allow developers to easily call all available integration tests in one mvn command.

I'm offering to submit a pull request to take a stab at such things once enough developers tell me what I should do and not do.
@pdurbin
Copy link
Member

pdurbin commented May 25, 2021

I think the smallest chunk is to factor the list of tests out of the script and into a plain text file. In the suggestion below, I put them in tests/integration-tests.txt:

(venv) beamish:dataverse pdurbin$ git diff --staged
diff --git a/conf/docker-aio/run-test-suite.sh b/conf/docker-aio/run-test-suite.sh
index bf0683fdb..ccc04733c 100755
--- a/conf/docker-aio/run-test-suite.sh
+++ b/conf/docker-aio/run-test-suite.sh
@@ -6,6 +6,9 @@ if [ -z "$dvurl" ]; then
        dvurl="http://localhost:8084"
 fi
 
+# TODO: Don't assume script will be called from root directory.
+tests=`cat tests/integration-tests.txt`
+
 # Please note the "dataverse.test.baseurl" is set to run for "all-in-one" Docker environment.
-# TODO: Rather than hard-coding the list of "IT" classes here, add a profile to pom.xml.
-source maven/maven.sh && mvn test -Dtest=DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT,AuxiliaryFilesIT -Ddataverse.test.baseurl=$dvurl
+# TODO: Rather than getting the list of "IT" classes here from a file, add a profile to pom.xml.
+source maven/maven.sh && mvn test -Dtest=$tests -Ddataverse.test.baseurl=$dvurl
diff --git a/tests/integration-tests.txt b/tests/integration-tests.txt
new file mode 100644
index 000000000..d6edfa4d7
--- /dev/null
+++ b/tests/integration-tests.txt
@@ -0,0 +1 @@
+DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT,AuxiliaryFilesIT
(venv) beamish:dataverse pdurbin$

@poikilotherm
Copy link
Contributor

poikilotherm commented May 27, 2021 •
edited
Loading

I vote for refactoring the whole thing. A tale I told before and would be great to see momentum. I have a strong opinion on not using any external files etc to define a list but start using the JUnit/Maven builtin mechanics properly.

  1. We are using the Surefire Plugin for Integration Tests (currently only in flavor "API End-To-End"). This is a violation of best practices, see StOv
  2. Using maven-failsafe-plugin, we tie the integration tests to mvn verify, not mvn test phase, effectively separating unit and integration tests. Skipping unit tests when running ITs only requires adding -DskipUnitTests (had been introduced for TestContainers profile already)

What we should do beyond that:

(We could also create proper test suites with JUnit 4, see the examples, but it would be a good thing to go for JUnit 5 anyway... 😉 )

@donsizemore
Copy link
Contributor Author

@poikilotherm I've already started down that path in a branch: https://github.com/OdumInstitute/dataverse/blob/run_test_suite/pom.xml#L844

In the mean time, IQSS would prefer to maintain its own list of preferred tests, so automated testing could reference that list while mvn verify might search **/*.java or some such until the ITs are refactored?

donsizemore pushed a commit to uncch-rdmc/dataverse that referenced this issue May 28, 2021
IQSS#7897 add phil's authoritative list of ITs for docker-aio and dat…
cdb7db4 
…averse-ansible
@donsizemore donsizemore mentioned this issue May 28, 2021
Merged
pdurbin added a commit that referenced this issue May 28, 2021
@pdurbin
mention new IT test list in dev guide #7897
f2a4d46 
Also remove reference to phoenix server, which has been decommissioned.
We'll finished cleaning up phoenix-related docs in #7031.
@pdurbin pdurbin mentioned this issue May 28, 2021
Merged
donsizemore added a commit to uncch-rdmc/dataverse that referenced this issue May 28, 2021
@donsizemore
Merge pull request #83 from IQSS/7897-docs
8d7d679 
mention new IT test list in dev guide IQSS#7897
@donsizemore donsizemore mentioned this issue Jun 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add Phil's authoritative list of ITs for docker-aio and dataverse-ansible uncch-rdmc/dataverse
3 participants
@pdurbin @poikilotherm @donsizemore