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

Issue on make test #399

Open
king-11 opened this issue Oct 17, 2020 · 9 comments
Open

Issue on make test #399

king-11 opened this issue Oct 17, 2020 · 9 comments

Comments

@king-11
Copy link
Contributor

king-11 commented Oct 17, 2020

So I ran tests on my system using make test but it failed on this test file tests/unit/test_routes/test_resource_update.py although it passed on circle ci
the error is as follows

INTERNALERROR>   File "/usr/local/lib/python3.7/site-packages/coverage/sqldata.py", line 1008, in _connect
INTERNALERROR>     self.con = sqlite3.connect(filename, check_same_thread=False)
INTERNALERROR> sqlite3.OperationalError: unable to open database file
/usr/local/lib/python3.7/site-packages/_pytest/stepwise.py:122: PytestCacheWarning: could not create cache path /src/.pytest_cache/v/cache/stepwise
  self.config.cache.set("cache/stepwise", [])
/usr/local/lib/python3.7/site-packages/_pytest/cacheprovider.py:399: PytestCacheWarning: could not create cache path /src/.pytest_cache/v/cache/nodeids
  config.cache.set("cache/nodeids", sorted(self.cached_nodeids))
/usr/local/lib/python3.7/site-packages/_pytest/cacheprovider.py:353: PytestCacheWarning: could not create cache path /src/.pytest_cache/v/cache/lastfailed
  config.cache.set("cache/lastfailed", self.lastfailed)
@aaron-junot
Copy link
Member

I think I've seen that before, but I also thought we fixed it... What commit was this a problem on? I don't have a problem on f971973 which is the HEAD of the repo as of 22 minutes ago

@king-11
Copy link
Contributor Author

king-11 commented Oct 20, 2020

@aaron-suarez i did find the error on 34e7f70 just pulled the latest changes from main

@Kandeel4411
Copy link
Collaborator

@aaron-suarez While we fixed it on the CircleCI pipeline by changing ownership of the current directory to the docker user, this issue I am afraid shall still pop up when run locally using docker-compose ( if the host you are cloning too has strict permissions about the current directory ) - possible temporary solution is to run sudo chown -R 5000 . locally (UID is per our current dockerfile).

Though in order for us to solve this permanently is to either remove the volume connection from the docker-compose configuration which will make it so that any changes in the source files will need rebuilding of the docker image ( I think we already do that anyway with our make config so it might not be much of a negative hassle ) or the other solution is to have different docker-compose files for production and development, where the development config will run as root user and still have the volume connected for ease of usage.

What do you guys think? If there are other solutions I am not aware of that you think would be better then I'd love to know!

@aaron-junot
Copy link
Member

I think I'd prefer the solution of running as root for development but I hope that problems don't pop up in staging that we couldn't catch in dev because of this difference. But I think it would probably be fairly easy to identify because it would certainly be permissions errors.

@Kandeel4411
Copy link
Collaborator

We could still run the CI pipeline using the production config since we can solve the permission problems easily there and it should hopefully catch any errors I think

@aaron-junot
Copy link
Member

Yeah, I think that will work. I trust our unit tests for the most part. Only once have I been caught up where I would have expected us to have a unit test for something and we didn't find out that the test was missing until production. And once in a 4 year project isn't too bad I think...

Ok so anyone that wants to handle this issue. Acceptance criteria:

  • create a new docker-compose file for development that runs as root and still have the volume connected
  • modify the current docker-compose file to run as a non-root user for CI and production
  • CI runs the production config (as non-root)

Did I cover everything @Kandeel4411 ?

@Kandeel4411
Copy link
Collaborator

Kandeel4411 commented Oct 21, 2020

Yup I think thats about everything - also what kind of config would our Makefile use? I think we need to include an env variable in there or something which is by default dev config but changed to prod on CI

@aaron-junot
Copy link
Member

The main "run" command in the Makefile is make run but that uses the flask development server. I have another command called make run-with-metrics that uses uwsgi instead of the flask server. I would think it makes most sense to just make additional commands similar to that, yeah?

So like, we have make test and we can have a make test-ci for when you need the production config. What do you think of that?

@Kandeel4411
Copy link
Collaborator

yeah that makes sense, that should do it I think

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

No branches or pull requests

3 participants