D61-IA / stellar-gnosis

Gnosis paper management and collaboration tool
Apache License 2.0
0 stars 1 forks source link

Feature/flaggingcomments #33

Closed Zhenghao-Zhao closed 4 years ago

Zhenghao-Zhao commented 4 years ago

Feature:

  1. Fix popup flagging form; 'thank you' message popup;
  2. Remove flagging page redirect.
  3. Add flagging icon with tooltip, filled for flagged comments, unfilled for unflagged comments.
  4. Filled icons are not clickable; unfilled ones opens flagging form if clicked.

File changes:

  1. Add css styling to flagging.css;
  2. Modify paper_detail to include necessary ids and classes;
  3. Add test_selenium_flaggingcomments.py that contains all selenium tests for this feature.
  4. Modify flagging.js functions, remove unused ones.

To run selenium tests, first install selenium: https://pypi.org/project/selenium/ then execute in your cmd: python manage.py test catalog.tests.test_selenium_flaggingcomments

Zhenghao-Zhao commented 4 years ago

Hi @Zhenghao-Zhao

thanks for updating. It looks close to ready. Have a look at a couple of my comments that require small updates.

Also, I noticed that when I run the selenium tests alone using,

python manage.py test catalog.tests.test_selenium_flaggingcomments

all works well. However, if I try to run all the tests together, Selenium and Django, using

python manage.py test catalog.tests

the Django-ones run first without a problem but the Selenium tests fail unable to login the user. The web browser opens and at the login page it give the error,

The username and/or password you specified are not correct.

Any ideas as to why this might happen?

Thank you!

I am not sure.

Here is what I found that might be related to this issue:

"If your tests rely on database access such as creating or querying models, be sure to create your test classes as subclasses of django.test.TestCase rather than unittest.TestCase. Using unittest.TestCase avoids the cost of running each test in a transaction and flushing the database, but if your tests interact with the database their behavior will vary based on the order that the test runner executes them. This can lead to unit tests that pass when run in isolation but fail when run in a suite." (https://docs.djangoproject.com/en/3.0/topics/testing/overview/)

My understanding is that in django.test.testcase all tests run with a mockup database, where as in unittest.testcase it runs in the same production database (default). Running them at the same time might have mixed the database those tests are run in therefore the error. However, after I changed my base test class to django.test.TestCase, I still got the same issue.

Maybe I will create a separate folder to store all selenium tests based on unittest instead.

PantelisElinas commented 4 years ago

@Zhenghao-Zhao

It looks good, and I will merge into develop.

With regards to the tests failing when run all together, your suggestion on separating the tests into two folders so we can run them separately sounds good. It will also solve the problem that Selenium tests need the website running whereas the Django tests don't. I will create a ticket for this and assign it to you to have a go at it. It is important to resolve this issue early in the development cycle.

Regards,

P.