Uploaded image for project: 'IGB'
  1. IGB
  2. IGBF-3626

Write test cases for UCSC Rest module

    Details

    • Type: Task
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Attachments

        Issue Links

          Activity

          Hide
          jsirigin Jaya Sravani Sirigineedi added a comment -

          Written test cases to test all methods in RestApiDataProvider class, currently working on UCSCRestSymLoader, written a test case but it isn't working when I create the Symloader directly, investigating a way to make it work. Also, found a way to debug the application using IntelliJ run configuration: Add a JAR file configuration with VM options, copy and paste all the options available in run_igb.sh file into the VM options.

          Show
          jsirigin Jaya Sravani Sirigineedi added a comment - Written test cases to test all methods in RestApiDataProvider class, currently working on UCSCRestSymLoader, written a test case but it isn't working when I create the Symloader directly, investigating a way to make it work. Also, found a way to debug the application using IntelliJ run configuration: Add a JAR file configuration with VM options, copy and paste all the options available in run_igb.sh file into the VM options.
          Hide
          jsirigin Jaya Sravani Sirigineedi added a comment -

          Resolved the issue with the UCSCRestSymloader test case by adding relevant dependencies in pom for the test phase alone. Code is available at the branch https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3501. I haven't done any mocking for the third-party APIs in the test phase as I didn't see it anywhere else, but I think mocking the UCSC Rest APIs for testing would be beneficial for us as it only checks our logic instead of checking whether the UCSC API is working or not Nowlan Freese, could you please let me know your thoughts on this.

          Show
          jsirigin Jaya Sravani Sirigineedi added a comment - Resolved the issue with the UCSCRestSymloader test case by adding relevant dependencies in pom for the test phase alone. Code is available at the branch https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3501 . I haven't done any mocking for the third-party APIs in the test phase as I didn't see it anywhere else, but I think mocking the UCSC Rest APIs for testing would be beneficial for us as it only checks our logic instead of checking whether the UCSC API is working or not Nowlan Freese , could you please let me know your thoughts on this.
          Hide
          nfreese Nowlan Freese added a comment -

          Please mock the APIs.

          We have had issues in the past where our tests rely on various online services being available (UCSC DAS), and when those services go down our builds fail, which is a problem.

          Jaya Sravani Sirigineedi - Do you think it would be beneficial to include some tests that do pull from the UCSC REST API? It would be nice to have a test that would fail if UCSC decided to change their API. However, I would want the test failure to lead to a warning and not an error, as I would still want the build to succeed in the event that the API was down.

          Show
          nfreese Nowlan Freese added a comment - Please mock the APIs. We have had issues in the past where our tests rely on various online services being available (UCSC DAS), and when those services go down our builds fail, which is a problem. Jaya Sravani Sirigineedi - Do you think it would be beneficial to include some tests that do pull from the UCSC REST API? It would be nice to have a test that would fail if UCSC decided to change their API. However, I would want the test failure to lead to a warning and not an error, as I would still want the build to succeed in the event that the API was down.
          Hide
          jsirigin Jaya Sravani Sirigineedi added a comment -

          Sure Nowlan Freese, I will work on mocking the API. I am not sure whether the other thing can be done, writing tests to check the APIs alone and only get a warning instead of an error, but I will investigate to see if there's any other way to do this.

          Show
          jsirigin Jaya Sravani Sirigineedi added a comment - Sure Nowlan Freese , I will work on mocking the API. I am not sure whether the other thing can be done, writing tests to check the APIs alone and only get a warning instead of an error, but I will investigate to see if there's any other way to do this.
          Hide
          jsirigin Jaya Sravani Sirigineedi added a comment - - edited

          While working on mocking the API, I found a problem with the com.google.common.io.Resources library which is used to make the call to UCSC Rest APIs in the module. I have used it to make the HTTP calls as I have seen it being used in few places in the project, but I found that this library isn't made to make HTTP calls, so I investigated other options to refactor the code to use a proper HTTP library. The options I came across are Apache httpclient library and default java.net httpclient which is present in java 11 or higher, after careful consideration, I decided to use the Apache one, here are the reasons:

          Why not Resources:

          Why not default java.net httpclient:

          • Although the default java.net httpclient provides a simpler way to call an API and doesn't require adding any dependency to the project, it doesn't provide more functionality than the dedicated Apache httpclient library provides.
          • The main advantage of this over the Apache library is the asynchronous http communication, which as per my investigation is not needed in our project.

          Why Apache httpclient:

          • Apache httpclient is made for making HTTP calls alone, so it is feature-rich and provides more convenient ways to make HTTP calls and handle exceptions.
          • The URL building is also easier by using the URIBuilder provided in the library.

          Refactoring is done to use the Apache httpclient and the code is available at https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3501.

          Show
          jsirigin Jaya Sravani Sirigineedi added a comment - - edited While working on mocking the API, I found a problem with the com.google.common.io.Resources library which is used to make the call to UCSC Rest APIs in the module. I have used it to make the HTTP calls as I have seen it being used in few places in the project, but I found that this library isn't made to make HTTP calls, so I investigated other options to refactor the code to use a proper HTTP library. The options I came across are Apache httpclient library and default java.net httpclient which is present in java 11 or higher, after careful consideration, I decided to use the Apache one, here are the reasons: Why not Resources: Firstly, the library isn't made to make HTTP calls, it is primarily made to work with resources on the classpath Because of that, it doesn't provide features or functionality to handle HTTP exceptions properly, for more information check: https://guava.dev/releases/21.0/api/docs/com/google/common/io/Resources Why not default java.net httpclient: Although the default java.net httpclient provides a simpler way to call an API and doesn't require adding any dependency to the project, it doesn't provide more functionality than the dedicated Apache httpclient library provides. The main advantage of this over the Apache library is the asynchronous http communication, which as per my investigation is not needed in our project. Why Apache httpclient: Apache httpclient is made for making HTTP calls alone, so it is feature-rich and provides more convenient ways to make HTTP calls and handle exceptions. The URL building is also easier by using the URIBuilder provided in the library. Refactoring is done to use the Apache httpclient and the code is available at https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3501 .
          Hide
          jsirigin Jaya Sravani Sirigineedi added a comment -

          Refactored the test cases to mock the UCSC APIs, and also did some refactoring to the HTTP call logic to make the code more readable and testable. Nowlan Freese For testing the UCSC APIs alone without making the test fail, I have refactored the existing EndPointTest class. Code is available at https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3501.

          Show
          jsirigin Jaya Sravani Sirigineedi added a comment - Refactored the test cases to mock the UCSC APIs, and also did some refactoring to the HTTP call logic to make the code more readable and testable. Nowlan Freese For testing the UCSC APIs alone without making the test fail, I have refactored the existing EndPointTest class. Code is available at https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3501 .
          Hide
          nfreese Nowlan Freese added a comment -

          Tested on Mac, tests are working as expected.

          I have included some notes below based on discussion with Sravani.

          In the project pom.xml, the following line is required:

          <configuration>
              <argLine>-XX:+EnableDynamicAgentLoading</argLine>
          </configuration>
          

          This is due to mockito using byte-buddy-agent which causes the warning in Java 21 - see https://openjdk.org/jeps/451 and https://github.com/raphw/byte-buddy/issues/1396

          Testing is successful with no internet for the two test files (UCSCRestSymloaderTest and RestApiDataProviderTest) - indicating the tests do not need access to the UCSC API (i.e. the results are being successfully mocked).

          Also modified is EndPointTest.java in the ExternalView plugin to test the UCSC API. If the API is not available then several warnings should be thrown, but the build will not fail.

          Show
          nfreese Nowlan Freese added a comment - Tested on Mac, tests are working as expected. I have included some notes below based on discussion with Sravani. In the project pom.xml, the following line is required: <configuration> <argLine>-XX:+EnableDynamicAgentLoading</argLine> </configuration> This is due to mockito using byte-buddy-agent which causes the warning in Java 21 - see https://openjdk.org/jeps/451 and https://github.com/raphw/byte-buddy/issues/1396 Testing is successful with no internet for the two test files (UCSCRestSymloaderTest and RestApiDataProviderTest) - indicating the tests do not need access to the UCSC API (i.e. the results are being successfully mocked). Also modified is EndPointTest.java in the ExternalView plugin to test the UCSC API. If the API is not available then several warnings should be thrown, but the build will not fail.
          Hide
          nfreese Nowlan Freese added a comment - - edited

          Please create a pull request for branch IGBF-3501 to main

          Show
          nfreese Nowlan Freese added a comment - - edited Please create a pull request for branch IGBF-3501 to main
          Hide
          jsirigin Jaya Sravani Sirigineedi added a comment -

          Created a Pull request for all the work that has been done till now on the UCSC rest module: https://bitbucket.org/lorainelab/integrated-genome-browser/pull-requests/990.

          Show
          jsirigin Jaya Sravani Sirigineedi added a comment - Created a Pull request for all the work that has been done till now on the UCSC rest module: https://bitbucket.org/lorainelab/integrated-genome-browser/pull-requests/990 .
          Hide
          ann.loraine Ann Loraine added a comment -

          I did not examine every change to the code, but what I did look at was very orderly and logical. I also like that all the changes now appear as a continuous "streak" of commits in the commit history. This makes it very easy to understand and follow all the changes. This will help a lot with writing about this work later, which I hope we can do before Jaya Sravani Sirigineedi completes her grad degree.

          Thank you Jaya Sravani Sirigineedi for your work !

          Show
          ann.loraine Ann Loraine added a comment - I did not examine every change to the code, but what I did look at was very orderly and logical. I also like that all the changes now appear as a continuous "streak" of commits in the commit history. This makes it very easy to understand and follow all the changes. This will help a lot with writing about this work later, which I hope we can do before Jaya Sravani Sirigineedi completes her grad degree. Thank you Jaya Sravani Sirigineedi for your work !
          Hide
          ann.loraine Ann Loraine added a comment -

          PR is merged.

          Show
          ann.loraine Ann Loraine added a comment - PR is merged.
          Hide
          ann.loraine Ann Loraine added a comment -

          Installers are built and ready for testing.

          Show
          ann.loraine Ann Loraine added a comment - Installers are built and ready for testing.
          Hide
          nfreese Nowlan Freese added a comment -

          Tested installer on Mac as well as tested the test files using the lorainelab main branch.

          Closing ticket.

          Show
          nfreese Nowlan Freese added a comment - Tested installer on Mac as well as tested the test files using the lorainelab main branch. Closing ticket.

            People

            • Assignee:
              jsirigin Jaya Sravani Sirigineedi
              Reporter:
              jsirigin Jaya Sravani Sirigineedi
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: