LabNeuroCogDevel/LNCDschedule

tests for AddNotes.py

Opened this issue · 22 comments

Similar to test_multiRA_insertions.py

suv27 commented

So I can understand this more, do you want unit test for the MultiRA.py python file? If this is correct, I would like to work on it and can you please assign it to me?

@Physsix27 Thanks for the interest! The issue is for AddNotes.py.

While the repo is open source and any help is welcome, LNCDschedule's scope is rather specific: schedule and record visits for the laboratory of neurocognative development. The integrations are limited to our api keys (qualtrics, gcal) or within our firewall (psql) and the schema is specific to the particular needs of the lab. So, not too useful to the world at large.

I feel like we'd be taking a generous resource from projects that have something more to offer the open source community. But, if that doesn't moderate your enthusiasm, I'll happily assign the issue to you!

suv27 commented

That shouldn't be a problem, yes please assign it to me.

suv27 commented

Hey @WillForan, would it be ok to create tests/test_AddNotes.py and write my tests in that file?
Just would like to know if you would like that structure.

There is some effort to separate db and ui tests into their own directories. so tests/ui/test_AddNotes.py might be the most appropriate place.

suv27 commented

Can you please walk me through how you run your tests and how do you install all the requirements needed?

Good question! I made it issue #32. tests/readme.md now has a bit more info.
Essentially you need all the dependencies + pytest fixtures. pdbpp also make debugging tests nicer.

python3 -m pip install --user PyQt5 pyesql psycopg2 'git+https://github.com/LabNeuroCogDevel/LNCDcal.py#egg=9592386' pyOpenSSL pytest-qt pytest-pgsql sqlalchemy pdbpp

and then run pytest on the test file

python3 -m pytest --pdb tests/ui/test_AddNotes.py

As an aside, I noticed (at least for me on archlinux) I need to install the actual database software postgresql for the python package pytest-pgsql's initdb to run successfully

whoa! there are more tests (and they're better organized) than what we have in this repo.

My instructions above failed to mention pytest should be run from the root directory of this repo ($PWD ends with LNCDschedule/). Looks like you might be running from the home directory?

suv27 commented

Hey, I am so sorry lol. I missed two projects that I am currently working on, that picture does not reference your repo.
Now I tried again in the correct repo, sorry again, and this is the error that I am facing.
image

Hmm. One error looks like LNCDcal depend is missing.
The local lncdSql can fail to load for similar depend issues. That error is thrown a lot. Maybe try python -c 'import lncdSql' to see why it's unhappy

suv27 commented

Alright, so I will be making a PR to fix and make everything easier to set up, as I found the problem.
This PR will contain a new requirements.txt and a modification to the Makefile and it looks like there is a need for cleaning the project too since it has a lot of lint error and that is one of the targets in you Makefile . You can make that an issue to clean the project with lint and assign it to me. and I will work on that after I put this PR about the makefile

suv27 commented

Hey I want to let you know that that command did not work, I am still getting that error for lncdSql

Thanks for the pull request! I'd not used coverage before, and a requirements.txt is long overdue (pipenv+Pipefile was abandoned).

I just loaded the project on a fresh install (using make install! Thanks for the renewed makefile!) and didn't hit any unexpected test failures/import errors. However, I did notice:

 python3 -m pytest tests/db/test_00_insert_update.py 

runs but

pytest tests/db/test_00_insert_update.py 

yields an error

E ModuleNotFoundError: No module named 'lncdSql'

suv27 commented

that is the error that I keep getting, IDK where that is coming from E ModuleNotFoundError: No module named lncdSql

do you get the import error for both invocations: pytest ... and python3 -m pytest ... ?

Looks like a PYTHONPATH issue also resolved with

 PYTHONPATH=$(pwd) pytest tests/db/test_00_insert_update.py 

https://stackoverflow.com/questions/10253826/path-issue-with-pytest-importerror-no-module-named-yadayadayada

suv27 commented

just for the pytest ... if we can get that fix that would make our life easier and then we can start writing tests for all the missing python files.
image

I think I'm missing something. Does pytest do anything better than python -m pytest?
(besides the extra typed characters, but then make test is shorted than pytest tests :) )

suv27 commented

of course make test is shorter make test is a target in the make file that contains a piece of sh code, with right now contains -> python3 -m pytest tests -v but if we make that fix it can be better to use the pytest straight. with pytest, we have more control and we have more features that we can use and make our coverage report easier to handle.

It looks like export PYTHONPATH="$PYTHONPATH:$(pwd)" will get pytest working without running as a module.

But any arguments you'd give to pytest will work the same with python -m pytest (e.g. python -m pytest --help is equivalent to pytest --help -- the main function sees the same sys.args). Coverage also supports running code as a module: coverage run -m pytest tests/

The baggage of using python3 -m .... (in addition to getting PWD onto the path for free) comes from debian and centos still using python2 as the default python. Though not explicitly an issue here

suv27 commented

so what do you suggest? where should we go from here? one thing I want to point out is that the coverage is not printing 100% correctly, is printing all the env files which suppose to not do that, it only suppose to do the python files we are testing

ahh yeah check out the Makefile I pushed a few minutes ago! --omit seems to do the trick.

I didn't finish reading about coverage config file. That might look a bit nicer, but I think its good enough for now or at least until the call gets a few more flags

suv27 commented

I see you made a lot of changes to it.
I ran make test and there were errors but yes I can definitely start tests files that are missing