Vis4Sense / HistoryMap

http://sensemap.io
60 stars 7 forks source link

Unit testing for 'Node - New node' #71

Closed kaidatavis closed 6 months ago

kaidatavis commented 7 years ago

https://goo.gl/8zPGzT

  1. Make sure you understand the behaviours described on the wiki page. Let me know if it is not clear or you don’t agree (for example you think this is not the best way to help user understand what happened);
  2. Create (Jasmine) 'test specs’ for these behaviours (up to the ‘it’ level, but not the function inside it), following Phong’s folder structure; 1a: for the ‘unclear’ cases, just use the current SenseMap behaviour;
  3. Add the actual javascript and sinon-chrome code to make the test running;
  4. For each behaviour, create different types of testing instances. Using ‘new node - manually enters a URL (in the address bar)’ as an example, possible instance can be:
    • url for different websites
    • different protocols: http, https,
    • without protocol
    • invalid url
  5. There is an extra challenge to figure out how to detect page update without url change.
RedayY commented 7 years ago

Right on it :)

RedayY commented 7 years ago

Update 1: Made a specification document for: -url for different websites -different protocols: http, https, -without protocol -invalid url

kaidatavis commented 7 years ago

Good to see the progress. I got your test running, and below is the screen shot. Is this correct?

screen shot 2017-06-21 at 11 03 07

kaidatavis commented 7 years ago

One small thing: please update the 'options.html' page to include link to your tests, and keep the links of other's test, such as Phong's, on the same options.html page. So we have a options.html with links to everyone's test. See the options.html in the latest 'rebuild' commit.

kaidatavis commented 7 years ago

I see your branch is still based on an older version of rebuild branch. This can be an issue when you try to merge your change to the 'rebuild' branch when all the tests are created. I suggest you 1) make a copy of your new test files, and then 2) delete the current rebuild-reday branch, 3) create a new rebuild-reday branch from the latest rebuild version, and finally 4) copy back the test files to the new rebuild-reday branch.

RedayY commented 7 years ago

Yes, so I will go through the specification in the meeting. I made sure that test includes all cases ( No protocol, with protocol, random website, multiple links, changing links)

I will fix options and the branch in a min

RedayY commented 7 years ago

Update: Updated Branch to the latest version of Sensemap Rebuilt Adjusted Testing Specification to work with the latest rebuild version

Positive Notes: I like the structure of how the code was written. :)

kaidatavis commented 7 years ago

Hi Reday, I only realised that your have done most of the work long time ago :-)

kaidatavis commented 7 years ago

There are two small things: 1) keep the option page as /html/options.html, and then add link to your test pages (/reday/node.test.js and /reday/node-sinon-chrome.test.js). Eventually we want to have the test page from everyone linked from the options.html.

kaidatavis commented 7 years ago

2) I see the latest version is still randomly picking a url from the array. I suggested in the last meeting to change this to go through every url in the array during test. Also, create an 'it' for each type of url, such as 'invalid url', and have more than one url for each type.

RedayY commented 7 years ago

I Apologize, I haven't updated any of my code lately. I will submit changes tonight.

Next Update: Small changes to include individual smaller tests for links. Adding AJAX Detection test. (Experimental small thing to help with detecting website changes through ajax requests.) Added Sinnon Tests.

RedayY commented 7 years ago

Conclusion:

By doing Unit testing for new nodes, following "Issues" have been determined:

-Even when links are not properly made, Nodes will be created. This also happens with unfinished links

RedayY commented 7 years ago

Updated Code to match new way of testing.

Personal note: I am unsure if this way is suitable for testing. We create tabs by defining them rather than actually creating them.

kaidatavis commented 7 years ago

It got buried under many other things; just got a chance to look at it.

We create tabs by defining them

The test code creates 'tabInfo' object, which simulates the real object created during chrome update event. Is this what you mean? SenseMap will treat these just as normal chrome update event and behave accordingly. This way we can test it without actually open a tab or webpage.

kaidatavis commented 7 years ago

I think you got the idea of how to create tests, and it is good to see that you tested four different cases.

I don't see anywhere manul_url_spec.js is used. Please removed it if it is no longer needed.

Also, please use capital letter rather than underscore for long name, so 'sinon_new_url_spec.js' should be 'sinonNewUrlSpec.js'.

Finally, it will be good to make the test more general and strict. See the discussions here #72 from about 8 days ago. You may want to talk to Shahzaib, and this can be applied to all tests.

This is certainly the right direction, but not quite there yet. Please 1) check every attribute, not just 'title' (url in your case), is the same, and 2) do this for all the nodes in the array assuming the array length is not known.

kaidatavis commented 7 years ago

It seems that I messed up the code in your branch. There were a lot of conflicts when I checked out the latest version from rebuild-Reday, and the a few commits I made today were to fix them but it doesn't seem to work. Please continue any further changes with your commit on on Jul 29, 2017 (not the commits I made later) and I will see if I can remove them.

RedayY commented 7 years ago

Updated Code

-Removed unused old tests -Renamed URL test specification document -Changed URL test specification to check every attribute and simplified test using a loop to test all wanted url. Can now simply add urls to make testing efficient. Thanks to Shaz!