PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
199 stars 230 forks source link

apps/api testing script fixes #3284

Open infotroph opened 2 months ago

infotroph commented 2 months ago

3283 changed URLs inside test_pecanapi.sh from localhost:8000 to pecan.localhost, but that script is intended for use inside the Docker image (where localhost is the right hostname to use). However it appears the script wasn't fully working before that change, so let's use this issue to plan the next updates before rolling back any of #3283.

Summary originally posted by @Sweetdevil144 in https://github.com/PecanProject/pecan/issues/3283#issuecomment-2040513884 :

I think we would be better off with Partially reverting our changes from pecan.localhost to localhost:8000 with following points in consideration :

Below is a description of all test cases which have been causing problems with current Repository changes. (I plan to revert changes from pecan.localhost back to localhost:8000 and then confirm it once again whether these runs are successful or not) :

   test_that("Using incorrect username & password returns Status 401", {
  res <- httr::GET(
    "http://pecan.localhost/api/models/",
    httr::authenticate("wrong_username_user", "wrong_password_user")
  )
  # return 200 when it should return 401. (Auth errors)
  expect_equal(res$status, 401)
})

test_that("Not using username & password returns Status 401", {
  res <- httr::GET(
    "http://pecan.localhost/api/models/",
  )
  # return 200 when it should return 401. (Auth errors)
  expect_equal(res$status, 401)
})
test_that("Calling /api/inputs/{input_id} with valid parameters returns Status 200", {
  res <- httr::GET(
    paste0("http://pecan.localhost/api/inputs/", 99000000003),
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 200)
})

test_that("Calling /api/inputs/{input_id} with invalid parameters returns Status 404", {
  res <- httr::GET(
    "http://pecan.localhost/api/inputs/0",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 404)
})

test_that("Calling /api/inputs/{input_id}?filename={filename} with invalid parameters returns Status 404", {
  res <- httr::GET(
    "http://pecan.localhost/api/inputs/295?filename=random",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 400)
})
test_that("Calling /api/runs/{run_id}/graph/{year}/{yvar}/ with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://pecan.localhost/api/runs/99000000282/graph/2002/GPP",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 200)
})

test_that("Calling /api/runs/{run_id}/graph/{year}/{yvar}/ with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://pecan.localhost/api/runs/1000000000/graph/100/GPP",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 404)
})

test_that("Calling /api/runs/{run_id}/input/{filename} with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://pecan.localhost/api/runs/99000000282/input/sipnet.in",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 200)
})

test_that("Calling /api/runs/{run_id}/input/{filename} with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://pecan.localhost/api/runs/1000000000/input/randomfile",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 404)
})

test_that("Calling /api/runs/{run_id}/output/{filename} with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://pecan.localhost/api/runs/99000000282/output/2002.nc",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 200)
})

test_that("Calling /api/runs/{run_id}/output/{filename} with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://pecan.localhost/api/runs/1000000000/output/randomfile",
    httr::authenticate("carya", "illinois")
  )
  # No response
  expect_equal(res$status, 404)
})
# Old file : 
test_that("Calling /api/status returns Status 200", {
# SPace was present in the GET command leading to failure of this test
  res <- httr::GET(" http://pecan.localhost/api/status")
  expect_equal(res$status, 200)
})
test_that("Submitting XML workflow to /api/workflows/ returns Status 201", {
  # Utilizing absolute path for localhost docker testing instead of 'test_workflows/api.sipnet.xml'
  xml_string <- paste0(xml2::read_xml("apps/api/tests/test_workflows/api.sipnet.xml"))
  res <- httr::POST(
    "http://pecan.localhost/api/workflows/",
    httr::authenticate("carya", "illinois"),
    httr::content_type("application/xml"),
    body = xml_string
  )
  # No Response
  expect_equal(res$status, 201)
})

test_that("Submitting JSON workflow to /api/workflows/ returns Status 201", {
  Sys.sleep(2)
  json_workflow <- jsonlite::read_json("test_workflows/api.sipnet.json")
  res <- httr::POST(
    "http://pecan.localhost/api/workflows/",
    httr::authenticate("carya", "illinois"),
    body = json_workflow,
    encode='json'
  )
  # No Response
  expect_equal(res$status, 201)
})

test_that("Calling /api/workflows/{id}/file/{filename} with valid parameters returns Status 200", {
  res <- httr::GET(
    paste0("http://pecan.localhost/api/workflows/", 99000000031, "/file/", "pecan.CONFIGS.xml"),
    httr::authenticate("carya", "illinois")
  )
  # No Response
  expect_equal(res$status, 200)
})

test_that("Calling /api/workflows/{id}/file/{filename} with invalid parameters returns Status 404", {
  res <- httr::GET(
    "http://pecan.localhost/api/workflows/0/file/randomfile.txt",
    httr::authenticate("carya", "illinois")
  )
  # No Response
  expect_equal(res$status, 404)
})
infotroph commented 2 months ago

In case it helps, here's how I'm currently running tests. To be clear, this is just the first way I tried that worked and there may well be better approaches.

The key reason for the complexity in my approach is that the default Docker Compose configuration doesn't run the API image from a shared volume, meaning any changes you make in your local apps/api won't show up in pecan-api-1 until you rebuild and restart the image. It would be possible to change this, but I wanted to get the existing tests working with as few changes as possible.

infotroph commented 2 months ago

When I run the above against commit 1bf158238 (head of develop just before #3283 was merged), it runs and passes the tests for auth and formats, then hangs on the 3rd through last tests in inputs, and if I comment those out it passes all tests for models, PFTs, ping, and the first three tests of runs before throwing a series of errors about missing IDs.

Comparing URL patterns in the passing and failing tests of test.inputs.R and test.runs.R, it looks like maybe URLs that pass IDs as query parameters (e.g. http://localhost:8000/api/foo/?id=100) are working correctly, but those that include them as paths (e.g. http://localhost:8000/api/foo/100) are not. I haven't debugged this any further yet, but I don't think those failures are the fault of how we run the test script.

Sweetdevil144 commented 2 months ago

it looks like maybe URLs that pass IDs as query parameters (e.g. http://localhost:8000/api/foo/?id=100) are working correctly, but those that include them as paths (e.g. http://localhost:8000/api/foo/100) are not.

This seems to be correct in our case. When we don't employ usage of ID parameters, it does not even returns a response. However, we can still get a status response with usage of ID Parameters. I guess the main issue lies due to aging of our test cases and no being able to line up with our Latest API changes

Sweetdevil144 commented 2 months ago

Correct me if I'm wrong. In the auth section, I don't think we require authorisation to GET our models via :

res <- httr::GET(
    "http://pecan.localhost/api/models/",
    # Test Failed
)

However, Authentication is required on using :

res <- httr::GET(
    "http://localhost:8000/api/models/",
    # Test Passed
)
Sweetdevil144 commented 2 months ago

I still think that the below test is useless since no timeout/endpoint has been set for such responses within our test.inputs.R

test_that("Calling /api/inputs/{input_id} with invalid parameters returns Status 404", {
  res <- httr::GET(
    "http://localhost:8000/api/inputs/0",
    httr::authenticate("carya", "illinois")
  )
  # No Response. Stops the testing Permanently
  expect_equal(res$status, 404)
})

Another potential issue may be lack of eror handeling and res$statusCode updation within our search.inputs.R in rpecanapi as below :

if(!is.null(model_id)) {
    url <- paste0(url, "&model_id=", model_id)
  }
  if(!is.null(site_id)) {
    url <- paste0(url, "&site_id=", site_id)
  }
  if(!is.null(format_id)) {
    url <- paste0(url, "&format_id=", format_id)
  }
  if(!is.null(host_id)) {
    url <- paste0(url, "&host_id=", host_id)
  }

I'm still not a 100% sure about this idea so please correct me if I'm not going thoroughly.

Sweetdevil144 commented 2 months ago

Status Update for test.input.R :

test_that("Calling /api/inputs/{input_id}?filename={filename} with valid parameters returns Status 200", {
  res <- httr::GET(
    paste0("http://localhost:8000/api/inputs/295?filename=fraction.plantation"),
    httr::authenticate("carya", "illinois")
  )
  # No response. Stops the testing Permanently
  expect_equal(res$status, 200)
})

test_that("Calling /api/inputs/{input_id}?filename={filename} with invalid parameters returns Status 404", {
  res <- httr::GET(
    "http://localhost:8000/api/inputs/295?filename=random",
    httr::authenticate("carya", "illinois")
  )
  # No Response. Stops the testing Permanently
  expect_equal(res$status, 400)
})
test_that("Calling /api/inputs/{input_id} with valid parameters returns Status 200", {
  res <- httr::GET(
    paste0("http://localhost:8000/api/inputs/", 99000000003),
    httr::authenticate("carya", "illinois")
  )
  # No Response. Stops the testing Permanently
  expect_equal(res$status, 200)
})

Are the related endpoints correctly created and linked? If not, it would be natural to return No Response. If yes, there may be any underlying bug in our rpecanapi causing this.

Sweetdevil144 commented 2 months ago

Just a preview from our Swagger Documentations about responses. Making sure the current endpoints and documentations are up to date.


Screenshot 2024-04-10 at 3 05 55 AM Screenshot 2024-04-10 at 3 04 26 AM

EDIT : Another Point to be Noted : No endpoint such as /api/inputs/{input_id}?filename={filename} exists within our documentation. Leaving No possible means to confirm the scenarios of testing.

Screenshot 2024-04-10 at 3 12 06 AM

Maybe these lines from our download.input.R from rpecanapi may help :

expr = {
      url <- paste0(server$url, "/api/inputs/", input_id)
      if(! is.null(filename)) {
        url <- paste0(url, "?filename=", filename)
      }

      if(! is.null(server$username) && ! is.null(server$password)){
        res <- httr::GET(
          url,
          httr::authenticate(server$username, server$password)
        )
      }
      else{
        res <- httr::GET(url)
      }
    }
    # Rest of Code
Sweetdevil144 commented 2 months ago

This is the test case I've been looking into for now in apps/api/tests/test.runs.R:

test_that("Calling /api/runs/{run_id}/graph/{year}/{yvar}/ with valid inputs returns Status 200", {
  res <- httr::GET(
    "http://localhost:8000/api/runs/99000000282/graph/2002/GPP",
    httr::authenticate("carya", "illinois")
  )
  # No Response
  expect_equal(res$status, 200)
})

I was looking within our SwaggerUI link and found its utilisation URL as follow : /api/runs/{run_id}/graph/{year}/{y_var} . Seems good, yet returns no response because the redirection URL in example test was as follow within our SwaggerUI : https://pecan-dev.ncsa.illinois.edu/api/runs/99000000282/graph/2002/GPP?x_var=time&width=800&height=600.

Two points I wanted clarity on :

Sweetdevil144 commented 2 months ago

Any Updates/Reviews @infotroph and @mdietze ??

Sweetdevil144 commented 2 months ago

Final briefing of all failing test cases with error logs/responses can be viewed in this test-branch's folder: https://github.com/Sweetdevil144/pecan/tree/test-branch/apps/api/tests

Started with debugging the corresponding rpecanapi folders and code files.

Sweetdevil144 commented 2 months ago

Final summary of total passing results {excluding failing(commented out) tests suites} :

| F W S  OK | Context
✔ |         3 | Testing authentication for API [96.2s]                                                                            
✔ |         4 | Testing all formats related endpoints [109.1s]                                                                    
✔ |         2 | Testing all inputs related endpoints [127.1s]                                                                     
✔ |         4 | Testing all models endpoints [13.6s]                                                                              
✔ |         4 | Testing all PFTs endpoints [13.7s]                                                                                
✔ |         1 | Testing the /api/ping endpoint                                                                                    
✔ |         3 | Testing all runs endpoints [7.3s]                                                                                 
✔ |         4 | Testing all sites endpoints [4.8s]                                                                                
✔ |         1 | Testing the /api/status endpoint [0.4s]                                                                           
✔ |         5 | Testing all workflows endpoints [10.3s] 
✔ |         5 | Testing all workflows endpoints [10.3s]                                                                           
⠏ |         0 | testthat                                                                                                          
══ Results ═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 389.7 s

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 31 ]
infotroph commented 2 months ago

Sorry for the long wait. It’s interesting that the suite takes so long even with failing tests commented out — are the long run times in the first three tests related to the auth issues you mentioned?

Sweetdevil144 commented 2 months ago

Sorry for the long wait. It’s interesting that the suite takes so long even with failing tests commented out — are the long run times in the first three tests related to the auth issues you mentioned?

I don't think so. It's just that my host machine has been bearing too much of a load recently and is underperformed. When it's only running docker, than it quickly finishes the test suites in about a minute or two.

allgandalf commented 2 months ago

I don't think so. It's just that my host machine has been bearing too much of a load recently and is underperformed.

If you decide to fix this, I can assign you a high end VM at my companies end for you to work faster, would that work for you ? @Sweetdevil144

Sweetdevil144 commented 2 months ago

If you decide to fix this, I can assign you a high end VM at my companies end for you to work faster, would that work for you ?

Yeah sure. No Problem with that. However, I think it isn't much big of a problem. I'd been running some high usage processes alongside pecan in docker at the time of testing, which caused the test to take lot of time. But I'd love to try the VM too.

infotroph commented 2 months ago

@GandalfGwaihir That's a generous offer! I trust that you

allgandalf commented 2 months ago

@infotroph , yeah no worries, also I confirm that this is an individual offer not related to PEcAn organisation!

@Sweetdevil144 ping me on slack with your system config, we're take it on personal DM from here

allgandalf commented 2 months ago

Assigned a VM , @Sweetdevil144 can you put in a timeline to solve this issue please ?

Sweetdevil144 commented 2 months ago

Hey, thanks a lot @GandalfGwaihir . It'll surely help a lot. As I estimate right now, this issue will be resolved by the end of this month. Although, I'll try to wrap it up as soon as possible. Also, Regarding this issue, @infotroph do you have any opinion regarding this thread? As far as it goes for the rpecanapi, I'm still debugging the roots of our issues. (Although I highly doubt what I presented above MAY be the reason for such high failure rates)