Enable integration tests

[forus]

• Fix and un-ignore most of tests that work with cbioportal database (aka integration tests)
• Document how to prepare cbioportal test database and run integration tests

[inodb]

@forus it looks like the Java CI is failing - is that expected?

Error: HTTP Status 403 for request POST https://api.github.com/repos/cBioPortal/cbioportal-core/dependency-graph/snapshots Error: Response body: { "message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest/dependency-graph/dependency-submission#create-a-snapshot-of-dependencies-for-a-repository" } Error: Resource not accessible by integration Error: HttpError: Resource not accessible by integration at /home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@octokit/request/dist-node/index.js:124:1 at processTicksAndRejections (node:internal/process/task_queues:95:5) at L (/home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@github/dependency-submission-toolkit/dist/index.js:6:1)

/home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@github/dependency-submission-toolkit/dist/index.js:7 ${JSON.stringify(n.response.data,void 0,2)})),n instanceof Error&&(a.error(n.message),n.stack&&a.error(n.stack)),new Error(Failed to submit snapshot: ${n}`)}} ^ Error: Failed to submit snapshot: HttpError: Resource not accessible by integration at L (/home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@github/dependency-submission-toolkit/dist/index.js:7:1) at processTicksAndRejections (node:internal/process/task_queues:95:5)

[forus]

@forus it looks like the Java CI is failing - is that expected?

Error: HTTP Status 403 for request POST https://api.github.com/repos/cBioPortal/cbioportal-core/dependency-graph/snapshots Error: Response body: { "message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest/dependency-graph/dependency-submission#create-a-snapshot-of-dependencies-for-a-repository" } Error: Resource not accessible by integration Error: HttpError: Resource not accessible by integration at /home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@octokit/request/dist-node/index.js:124:1 at processTicksAndRejections (node:internal/process/task_queues:95:5) at L (/home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@github/dependency-submission-toolkit/dist/index.js:6:1)

/home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@github/dependency-submission-toolkit/dist/index.js:7 ${JSON.stringify(n.response.data,void 0,2)})),n instanceof Error&&(a.error(n.message),n.stack&&a.error(n.stack)),new Error(Failed to submit snapshot: ${n}`)}} ^ Error: Failed to submit snapshot: HttpError: Resource not accessible by integration at L (/home/runner/work/_actions/advanced-security/maven-dependency-submission-action/v4.0.0/node_modules/@github/dependency-submission-toolkit/dist/index.js:7:1) at processTicksAndRejections (node:internal/process/task_queues:95:5)

@inodb Hi! No, that is not expected. I'm looking into it.

[forus]

@inodb It seems like a permission problem. The dependency graph snapshot could not be uploaded to GitHub.

Cold it be because the PR is created between organisations? se4bio -> cBioPortal

@sheridancbio gotten this issue as well https://github.com/cBioPortal/cbioportal-core/pull/25

[haynescd]

@inodb I think we should update the Java CI with maven job to only update dep. graph on push to main not PR

[forus]

Instead of IT and not IT names in front of the classes you should divide them into two directories it and <>. This will make the file names look nicer, show better organization inside the IDE, and help out with the way that the POM is setup.

Other then that this LGTM.

Hi Jeremy (@JREastonMarks),

Thank you for your feedback. I agree with you that it would be a better code organisation.

In fact, I tried to split directories to src/test/java and src/integration-test/java at the beginning. Unfortunately, I encountered challenges in getting this setup to function as intended.

In my most recent attempt, I used add-test-source and add-test-resource goals of build-helper-maven-plugin plugin to get integration tests to compile. However the plugin makes unit tests and integration tests to compile to the very same class directory. I did not find how can you compile them in separate folders. Consequently, it remains necessary to differentiate between unit tests and integration tests by naming convention, specifically excluding integration tests (*/IT) during the test phase and including them in the integration-test phase. Otherwise, test phase would pick up running integration tests. I found it even more confusing to both place integration tests in a separate folder and ensure their names match the pattern.

Below is the relevant plugin configuration used:

<plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>2.21.0</version>
                <executions>
                    <execution>
                        <id>default-test</id>
                        <phase>test</phase>
                        <goals>
                            <goal>test</goal>
                        </goals>
                        <configuration>
                            <excludes>
                                <exclude>**/IT*</exclude>
                            </excludes>
                        </configuration>
                    </execution>
                    <execution>
                        <id>integration-test</id>
                        <phase>integration-test</phase>
                        <goals>
                            <goal>test</goal>
                        </goals>
                        <configuration>
                            <includes>
                                <include>**/IT*</include>
                            </includes>
                            <forkCount>1</forkCount>
                            <reuseForks>false</reuseForks>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

    <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>build-helper-maven-plugin</artifactId>
                <version>3.2.0</version>
                <executions>
                    <execution>
                        <id>add-integration-test-sources</id>
                        <phase>generate-test-sources</phase>
                        <goals>
                            <goal>add-test-source</goal>
                        </goals>
                        <configuration>
                            <sources>
                                <source>src/integration-test/java</source>
                            </sources>
                        </configuration>
                    </execution>
                    <execution>
                        <id>add-integration-test-resources</id>
                        <phase>generate-test-resources</phase>
                        <goals>
                            <goal>add-test-resource</goal>
                        </goals>
                        <configuration>
                            <resources>
                                <resource>
                                    <directory>src/integration-test/resources</directory>
                                </resource>
                            </resources>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

Have you, or anyone else on the team, successfully implemented such a directory split before? If so, I'd greatly appreciate any insights or suggestions on how to resolve the issues encountered.

Given the improvements this PR brings to our current setup, I recommend proceeding with the merge. We can revisit and attempt a more distinct separation of test directories when we know more.

[JREastonMarks]

In fact, I tried to split directories to src/test/java and src/integration-test/java at the beginning. Unfortunately, I encountered challenges in getting this setup to function as intended.

I would do something like the following: src/test/java/org/mskcc/cbio/portal/integrationTest/pancancer/PanCancerStudySummaryImport.java

That way you aren't messing with the file organization which can make build systems unhappy.

[forus]

In fact, I tried to split directories to src/test/java and src/integration-test/java at the beginning. Unfortunately, I encountered challenges in getting this setup to function as intended.

I would do something like the following: src/test/java/org/mskcc/cbio/portal/integrationTest/pancancer/PanCancerStudySummaryImport.java

That way you aren't messing with the file organization which can make build systems unhappy.

@JREastonMarks

I see. So you propose to have a special integrationTest java package. That could work.

How about having src/test/java/integrationTest/org/mskcc/cbio/portal/pancancer/TestPanCancerStudySummaryImport.java instead or maybe even something like src/test/java/integrationTest/pancancer/TestPanCancerStudySummaryImport.java ?

I am not sure you could drop Test* in the name. We have helper classes that are not test classes e.g. org.mskcc.cbio.portal.pancancer.LargeTestSetGenerator

[JREastonMarks]

@forus The issue with doing at such a high level is that /src/test/[PACKAGE NAME] is that your code is in the IDE is going to be in multiple places as well as issues getting it to run. If you keep the code closer to the packages and areas that it tests and touches then it makes it easier for developers to work with it. So instead of src/test/java/integrationTest/org/mskcc/cbio/portal/pancancer/TestPanCancerStudySummaryImport.java I would recommend something like this src/test/java/org/mskcc/cbio/portal/pancancer/integrationTest/TestPanCancerStudySummaryImport.java or src/test/java/org/mskcc/cbio/portal/integrationTest/pancancer/TestPanCancerStudySummaryImport.java.

The big goal, outside of just having the tests run, is to make sure that they can easily be updated and accessed. We can write amazing tests but if people don't keep up with them or have a difficult time running them then they lose their purpose.

[forus]

@JREastonMarks Please have another look. Test classes names do not to follow any naming convention to run properly after all. The surefire plugin distinguish tests from utility test classes just fine and we don't need to help it in this. However we should follow a naming convention to make distinguishing test classes from production classes (testee) more easier. I renamed integration test: IT -> Test for readability purpose.

[forus]

After rebasing on the recent version of main branch, integration tests started to fail. I am looking into that.

[forus]

After rebasing on the recent version of main branch, integration tests started to fail. I am looking into that.

I've commented out the configuration that caused integration tests to fail