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.

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

chore: Add the faas-js-runtime to the testing suite. #377

Merged
merged 2 commits into from
Mar 3, 2023

Conversation

lholmquist
Copy link
Member

The faas-js-runtime is the framework which runs and invokes node.js serverless functions for Openshift Serverless.

@lholmquist
Copy link
Member Author

[test]

2 similar comments
@lholmquist
Copy link
Member Author

[test]

@lholmquist
Copy link
Member Author

[test]

@lholmquist
Copy link
Member Author

@zmiklank not sure what is going on with the tests. doesn't look like it has anything to do with what I've added

@pkubatrh
Copy link
Member

pkubatrh commented Feb 2, 2023

We are seeing issues with RHEL-based CI due to some of the infrastructure used for the tests getting refreshed over the coming week or two. I expect we will have to wait until that is done before the tests have a chance to get green.

What I find more troubling is that I do not see client test case results in the test runs that did pass. The only mention of them is this

Running tests for image quay.io/fedora/nodejs-18:18
-----------------------------------------------
Running test test_client_express (starting at 2023-02-02 14:03:19+00:00) ... 
-----------------------------------------------
Running express client test
Running express test suite
Please specify a valid test application

Rerunning the tests locally with more versbosity:

 echo 'Running test test_client_express (starting at 2023-02-02 17:22:17+01:00) ... '
Running test test_client_express (starting at 2023-02-02 17:22:17+01:00) ... 
+ echo -----------------------------------------------
-----------------------------------------------
+ test_client_express
+ echo 'Running express client test'
Running express client test
+ test_running_client_js express
+ echo 'Running express test suite'
Running express test suite
+ prepare express
+ image_exists quay.io/fedora/nodejs-18:18
+ docker inspect quay.io/fedora/nodejs-18:18
+ case "$1" in
+ [[  test_client_express
test_client_pino
test_client_prom
test_client_opossum
test_client_kube
test_client_faas
  =~ test_client_express  ]]
+ echo 'Please specify a valid test application'
Please specify a valid test application
+ exit 1
+ ct_cleanup
+ ct_show_resources
+ echo

This seems like a bug and the client cases are ignored completely. Also an error like this should have at least failed the whole test suite.

@pkubatrh
Copy link
Member

pkubatrh commented Feb 3, 2023

Immediate issues in the client test case runner will be fixed in #378

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pkubatrh
Copy link
Member

Now that #378 is merged, please rebase and lets try this once more.

The faas-js-runtime is the framework which runs and invokes node.js serverless functions for Openshift Serverless.
@lholmquist
Copy link
Member Author

rebased

@lholmquist
Copy link
Member Author

[test]

@lholmquist
Copy link
Member Author

Looks like these failures are unrelated to the client tests

@pkubatrh
Copy link
Member

The tests fail with test/test-lib.sh: line 1300: test_client_faas: command not found

Looking at the changes it seems like the test case wrapper function (something like https://github.com/sclorg/s2i-nodejs-container/blob/master/test/test-lib-nodejs.sh#L392) is missing.

@lholmquist
Copy link
Member Author

Ah yes, that part wasn't there when we added the previous clients. Added the function

@lholmquist
Copy link
Member Author

[test]

@lholmquist
Copy link
Member Author

Looks like the failures are unrelated to this PR.

@pkubatrh
Copy link
Member

Looks like more pino failures :/ Let me rerun failed jobs to see if they managed to change in the meantime.

@lholmquist
Copy link
Member Author

Since the failure doesn't have to do with this PR, could we get this merged and I will create another issue for our team to look at regarding the pinot tests?

@pkubatrh
Copy link
Member

pkubatrh commented Mar 2, 2023

RHEL8 tests for Node 18 failing on faas-js:

  ---
    operator: error
    at: Test.<anonymous> (/opt/app-root/src/test/test.js:459:13)
    stack: |-
      Error: expected 200 "OK", got 503 "Service Unavailable"
          at /opt/app-root/src/test/test.js:456:10
          at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      ----
          at Test._assertStatus (/opt/app-root/src/node_modules/supertest/lib/test.js:252:14)
          at /opt/app-root/src/node_modules/supertest/lib/test.js:308:13
          at Test._assertFunction (/opt/app-root/src/node_modules/supertest/lib/test.js:285:13)
          at Test.assert (/opt/app-root/src/node_modules/supertest/lib/test.js:164:23)
          at localAssert (/opt/app-root/src/node_modules/supertest/lib/test.js:120:14)
          at /opt/app-root/src/node_modules/supertest/lib/test.js:125:7
          at Request.callback (/opt/app-root/src/node_modules/superagent/lib/node/index.js:866:3)
          at /opt/app-root/src/node_modules/superagent/lib/node/index.js:1057:18
          at IncomingMessage.<anonymous> (/opt/app-root/src/node_modules/superagent/lib/node/parsers/json.js:21:7)
          at IncomingMessage.emit (node:events:525:35)
  ...
�[31mnot ok 96 should be strictly equal�[0m
  ---
    operator: equal
    expected: |-
      'OK'
    actual: |-
      '{"statusCode":503,"error":"Service Unavailable","message":"res.setHeader is not a function"}'
    at: Test.<anonymous> (/opt/app-root/src/test/test.js:460:13)
    stack: |-
      Error: should be strictly equal
          at Test.assert [as _assert] (/opt/app-root/src/node_modules/tape/lib/test.js:312:48)
          at Test.bound [as _assert] (/opt/app-root/src/node_modules/tape/lib/test.js:95:17)
          at Test.strictEqual (/opt/app-root/src/node_modules/tape/lib/test.js:476:7)
          at Test.bound [as equal] (/opt/app-root/src/node_modules/tape/lib/test.js:95:17)
          at Test.<anonymous> (/opt/app-root/src/test/test.js:460:13)
          at Test.assert (/opt/app-root/src/node_modules/supertest/lib/test.js:172:8)
          at localAssert (/opt/app-root/src/node_modules/supertest/lib/test.js:120:14)
          at /opt/app-root/src/node_modules/supertest/lib/test.js:125:7
          at Request.callback (/opt/app-root/src/node_modules/superagent/lib/node/index.js:866:3)
          at /opt/app-root/src/node_modules/superagent/lib/node/index.js:1057:18
  ...

Node 16 on RHEL8 failed due to what looks like infra issues, will try rerunning once more.
The rest is pino failures

@lholmquist
Copy link
Member Author

@pkubatrh is there a way to test that RHEL8 node 18 image locally?

i tried to do this podman run -it ubi8/nodejs-18:1 /bin/bash but i got an authentication error and i don't know where i'm suppose to authenticate to 🤷

@lholmquist
Copy link
Member Author

Also wanted to mention that i created an issue for the pino failures: #381

@lholmquist
Copy link
Member Author

[test]

@pkubatrh
Copy link
Member

pkubatrh commented Mar 3, 2023

@lholmquist You likely defaulted to the authenticated red hat registry (registy.redhat.io), try podman run -it registry.access.redhat.com/ubi8/nodejs-18:1 /bin/bash instead

@lholmquist
Copy link
Member Author

You likely defaulted to the authenticated red hat registry (registy.redhat.io),

@pkubatrh most likely

I did re-run the test suite and it looks like that issue where the faas-js client failed resolved itself. Still seeing those pino failures, but i created a new issue for that #381

@pkubatrh
Copy link
Member

pkubatrh commented Mar 3, 2023

Rerunning RHEL8 tests to see if faas-js manages to pass.

@pkubatrh pkubatrh self-requested a review March 3, 2023 14:28
@pkubatrh pkubatrh merged commit 3b9e360 into sclorg:master Mar 3, 2023
@lholmquist
Copy link
Member Author

🎉

@lholmquist lholmquist deleted the add-faas-js-runtime-to-testing branch May 2, 2023 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants