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

fix: failing unit tests #214

Closed
athith-g opened this issue May 17, 2024 · 7 comments
Closed

fix: failing unit tests #214

athith-g opened this issue May 17, 2024 · 7 comments

Comments

@athith-g
Copy link

Issue
23 unit tests are failing. 22 tests fail due to connecting to the wrong db port here and here (should be using 27017).

Reproduce
Run the following commands:

  1. pip install -r requirements.txt
  2. pip install -r requirements_dev.txt
  3. cd examples/petstore-access-control
  4. docker compose up -d
  5. cd ../..
  6. pytest tests
@athith-g athith-g mentioned this issue May 17, 2024
7 tasks
@uniqueg
Copy link
Member

uniqueg commented May 19, 2024

Can you please tell me where you had the tests failing? The CI pipeline seems to work fine (now that #218 was fixed).

@athith-g
Copy link
Author

Sorry I should have been more descriptive. The test in tests/security/test_cors.py is the only one that doesn't fail due to server timeouts. Tests in tests/security/access_control/test_access_control_server.py, tests/security/access_control/test_register_access_control.py, and tests/security/access_control/foca_casbin_adapter/test_adapter.py all fail due to timeouts.

Screenshot 2024-05-19 at 6 45 08 PM

If the tests are passing in the pipeline I may be doing something incorrectly. Is there a step missing in the reproduction steps?

@uniqueg
Copy link
Member

uniqueg commented May 20, 2024

Yes, I guess so. It appears that when the unit tests for Casbin access control stuff were written in FOCA, the connections to the database were not mocked. So the tests require a database to be up and running (you can also see that in the GitHub Actions workflow, it uses the mongodb-github-action).

To make the tests pass locally, I ran the following:

docker run --name mongodb -p 12345:27017 -d mongo:7.0

In my opinion, these are integration tests rather. Unit tests shouldn't have requirements like that. But I guess it's easy enough to fulfull the requirements, so I am not planning on changing it now.

Btw, I guess your fix in #215 worked for you because you probably happened to conincidentally have a MongoDB instance called mongodb running at port 27017 :)

@uniqueg
Copy link
Member

uniqueg commented May 20, 2024

Anyway, if you can confirm that this works for you (i.e., firing up a MongoDB instance at port 12345), then I think you can close this issue and the accompanying PR :)

@athith-g
Copy link
Author

That makes sense thank you for clearing things up!

Before I close the issue, I think the changes to tests/security/test_cors.py (15ba286) may still be valid. The reason it passes in the pipeline and not in my local environment is that the error only shows up in python 3.12. Even though the test passes in python 3.10, I don't think it's testing the desired functionality (i.e., the assertion is true even though the mock object wasn't called), as described here.

@uniqueg
Copy link
Member

uniqueg commented May 20, 2024

Thanks for pointing this out, good catch! 👍 But I think I have already dressed this in b1100bc, when I added support for Py3.12 last night. Can you please check and see if you agree on whether this fix addresses the underlying problem?

@athith-g
Copy link
Author

You definitely did my bad! Everything looks good.

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 a pull request may close this issue.

2 participants