-
Notifications
You must be signed in to change notification settings - Fork 241
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
Update API endpoint URLs #3283
Update API endpoint URLs #3283
Conversation
@Sweetdevil144 can you give some detail on how you tested this? I ask because this PR broke the test pathway I'm aware of, but maybe it enables another one I didn't know about? The pathway I know about is |
Sure. Apologies I forgot to test this in hurry. This won't happen again and I assure you of this. Although talking about some breaking changes, I think some of our pre-existing test cases may be either false or returning wrong output. After redirection to 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")
)
expect_equal(res$status, 401)
})
test_that("Not using username & password returns Status 401", {
res <- httr::GET(
"http://pecan.localhost/api/models/",
)
expect_equal(res$status, 401)
})
# Below is a response > res <- httr::GET(
+ "http://pecan.localhost/api/models/",
+ )
> res
Response [http://pecan.localhost/api/models/]
Date: 2024-04-05 13:34
Status: 200
Content-Type: application/json
Size: 3.58 kB
> res <- httr::GET(
+ "http://pecan.localhost/api/models/",
+ )
> res
Response [http://pecan.localhost/api/models/]
Date: 2024-04-05 16:20
Status: 200
Content-Type: application/json
Size: 3.58 kB
# Even though no username and password are provided, we still see a status returned 200. I tried navigating to this page and found a systematic api response too without any authentication.
> Is this a general error or should this be working as defined previously? I will test all the responses thoroughly and debug the errors too. |
This test takes too long to run. In short, the suites don't pass over this test : 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")
)
expect_equal(res$status, 200)
}) |
I'm taking further look into this |
ConclusionI think we would be better off with Partially reverting our changes from
Below is a description of all test cases which have been causing problems with current Repository changes. (I plan to revert changes from
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)
}) |
Let's plan the follow-up work in #3284 -- long comment threads in already-merged PRs can get confusing. |
This Fix is similar to this issue in our
rpecanapi
Repository : PecanProject/rpecanapi#21Description
Refracted server URLs to use localhost PEcAn URLs
Motivation and Context
Review Time Estimate
Types of changes
Checklist: