spacedeck / spacedeck-open

Spacedeck, a web based, real time, collaborative whiteboard application with rich media support
GNU Affero General Public License v3.0
1k stars 243 forks source link

Lack of automated tests. #88

Open MishkinBerteig opened 4 years ago

MishkinBerteig commented 4 years ago

Proposal: create an initial set of tests with Jest (compatible with Vue, one of the major dependencies of spacedeck).

There is substantial code in this project already. Lack of automated tests for that code causes three main risks to be larger than needed:

  1. Making changes is prone to causing unexpected side-effects.
  2. Refactoring is nearly impossible.
  3. Updating dependencies is fraught with risk.

Resolving this issues is a large amount of work. I suggest it be broken into four steps:

Step 1: create three to five trivial back end unit tests and a similar number of trivial front end tests. These tests will focus on basic aspects of spacedeck functionality.

Step 2: work on creating basic regression tests for the recently fixed star/starburst and multi-user cursor position issues.

Step 3: start driving all further work with test-first approach that backfills regression tests as various parts of the code are touched.

Step 4: work on systematically improving code coverage of the existing code base with automated tests.

This issue should be closed once the initial set of trivial tests is created in Step 1. Step 2 could be written up as one or two separate issues for tracking. Steps 3 and 4 are really separate projects, not just issues.

mntmn commented 4 years ago

Hi, I agree that testing is useful. I would prefer end-to-end integration tests that cover the most essential user stories first.

Example: "Log in, create a space, create a note, write text" or "Share space, open another browser window, check if expected space/content is visible".

If these are written with uncomplicated CSS selectors, the tests are not tightly coupled to the structure of the javascript code, so it can be refactored without having to rewrite tests all the time. I agree that jest is a good solution for this.

I will not enforce TDD in the project, but you are free to use it when introducing new features.

Unit tests and specialized regression tests are fine for algorithmic / rendering / coordinate translation and so on code parts, in my opinion.

JohnMcLear commented 3 years ago

Sauce labs provide free testing for Foss. Email them.

cynthia-rempel commented 3 years ago

Would selenium tests be helpful? They're really easy to write... Looks like selenium could export to mocha testing framework... Took me ~2 min to write a test to create an admin account, and draw a circle... test1.spec.zip

banglashi commented 3 years ago

Hi @cynthia-rempel yes, I think they would. Would you care doing an initial PR to get this started?

cynthia-rempel commented 3 years ago

Sure! I'll research the correct structure for a mocha test-suite, create some tests for it, and make a pull request :)

cynthia-rempel commented 3 years ago

I tried to create a pull request, and got a 403 error...

cynthia-rempel commented 3 years ago

Figured out how to create a mocha test harness, that runs the tests without erroring out... test-harness.zip Checked the install works... It does require chrome and chromedriver on the path to run. Like the original selenium tests, it's missing assertions...

cynthia-rempel commented 3 years ago

Ran a zap basline scan: docker run -v $(pwd):/zap/wrk/:rw -t owasp/zap2docker-stable zap-baseline.py -t http://172.17.0.1:9666 -g gen.conf -r testreport.html Got some output... testreport.zip

banglashi commented 3 years ago

Hi @cynthia-rempel

thanks for the infos, I haven't been able to test it yet, but will try to get some time soon for it. In the meantime, could I help you to fix the pull request? This way it would be easier to get your work integrated. I also got an account at sauce lab like @JohnMcLear suggested. @JohnMcLear do you have any experience with it, if you do, would you be willing to help set it up?

cynthia-rempel commented 3 years ago

Wow! Thanks! I had success with the .side files; I'm not 100% confident in my translation of the .side file to javascript... I think I'd like to polish the javascript a bit more (or possibly figure out another way to automate running .side files?

cynthia-rempel commented 3 years ago

I do have confidence in the harness itself though :)

cynthia-rempel commented 3 years ago

I created a user admin with a password then ran:

docker run --dns 9.9.9.9 -v $(pwd):/zap/wrk/:rw -t owasp/zap2docker-stable zap-full-scan.py -t http://admin:password@172.17.0.1:9666 -g gen.conf -r testreport.html

Produced a testreport.zip lot more results...

cynthia-rempel commented 3 years ago

Again thanks for helping me with the pull request :) I'll go back to polishing translating the javascript translations of the unit tests.

cynthia-rempel commented 3 years ago

Figured out how to run the .side file from the command line (as opposed to the browser :) ): docker run -d -p 4444:4444 -v /dev/shm:/dev/shm selenium/standalone-chrome:89.0 selenium-side-runner --server http://localhost:4444/wd/hub --base-url http://172.17.0.1:9666 -c "browserName=chrome goog:chromeOptions.args=[disable-infobars, headless]" test/create-admin2-user.side --debug create-admin2-user.side.zip

cynthia-rempel commented 3 years ago

It still would be nice to run them as mocha tests though...

cynthia-rempel commented 3 years ago

Polished the tests a little, added some profile and space tests. Having some issues with the page asking me if I really want to leave the page... user-test.zip

cynthia-rempel commented 3 years ago

I also noticed quite a few of the findings in the ZAP test report were false-positives related to spacedeck returning 200 codes instead of 404 errors... https://github.com/spacedeck/spacedeck-open/issues/186

cynthia-rempel commented 3 years ago

I think I might have figured out how to do a pull request! I'll start working towards getting a testsuite PR in the next two weeks :)

mntmn commented 3 years ago

Looking forward to the PR!