Difference between revisions of "OSD600 and DPS909 Winter 2018 Lab 6"
(→1. Get a Partner) |
(→8. Submission) |
||
| (46 intermediate revisions by 24 users not shown) | |||
| Line 1: | Line 1: | ||
=Fixing a Bug, Adding Tests= | =Fixing a Bug, Adding Tests= | ||
| − | This week we're looking at the [https://brave.com/ Brave browser]. We spent time working through a [https://github.com/humphd/browser-laptop/tree/good-first-experience-issue-10554#walkthrough-fixing-a-bug-in-the-brave-browser walkthrough of bug fix] together. Now it's your turn to try and fix a bug and add more tests. | + | This week we're looking at the [https://brave.com/ Brave browser]. We spent time working through a [https://github.com/humphd/browser-laptop/tree/good-first-experience-issue-10554#walkthrough-fixing-a-bug-in-the-brave-browser walkthrough of a bug fix] together. Now it's your turn to try and fix a bug and add more tests. |
In this lab, you are asked to fix a bug in Brave using [http://agiledata.org/essays/tdd.html Test Driven Development (TDD)]. With TDD, we write tests '''before''' we implement the code, and work toward getting our tests to pass. The goals of the lab include: | In this lab, you are asked to fix a bug in Brave using [http://agiledata.org/essays/tdd.html Test Driven Development (TDD)]. With TDD, we write tests '''before''' we implement the code, and work toward getting our tests to pass. The goals of the lab include: | ||
| Line 62: | Line 62: | ||
==4. Compare Implementations== | ==4. Compare Implementations== | ||
| − | Sometimes it's helpful to compare how | + | Sometimes it's helpful to compare how different projects implement the same feature. You can often learn a lot by reading the code in a project that does what you want, in order to make another do new things. |
We'll compare the code for Firefox, Chrome, and Brave: | We'll compare the code for Firefox, Chrome, and Brave: | ||
| Line 70: | Line 70: | ||
* The equivalent code in Chrome is also written in C++, see https://cs.chromium.org/chromium/src/components/url_formatter/url_fixer.cc and https://cs.chromium.org/chromium/src/components/url_formatter/url_fixer.h | * The equivalent code in Chrome is also written in C++, see https://cs.chromium.org/chromium/src/components/url_formatter/url_fixer.cc and https://cs.chromium.org/chromium/src/components/url_formatter/url_fixer.h | ||
| − | Read through both files and try to orient yourself. Don't get hung-up on understanding every single line, especially in the | + | Read through both files and try to orient yourself. Don't get hung-up on understanding every single line, especially in the C++ code, which uses lots of internal string APIs, [https://en.wikipedia.org/wiki/XPCOM XPCOM code], etc. Instead, try to find clues as to how each one is doing what it does with input strings before converting them to URLs. |
==5. Add Test Cases== | ==5. Add Test Cases== | ||
| Line 123: | Line 123: | ||
|- | |- | ||
| 1 | | 1 | ||
| − | | | + | | Yalong Li |
| − | | | + | | https://github.com/yalooong/browser-laptop/commit/d73de8762d853408294501d134cd5966ad232137 |
| − | | | + | | https://yalongxyz.blogspot.ca/2018/04/osd-lab6-blog-post.html |
|- | |- | ||
| 2 | | 2 | ||
| − | | | + | | Matthew Quan |
| − | | | + | | https://github.com/AlexWang-16/browser-laptop/commit/64526635e6e39e1dbadbf915e0010f8465170285 |
| − | | | + | | https://mattprogrammingblog.wordpress.com/2018/04/07/osd600-lab-6-fixing-a-bug-in-brave/ |
|- | |- | ||
| 3 | | 3 | ||
| − | | | + | | Kelvin Cho |
| − | | | + | | https://github.com/TheKinshu/browser-laptop/commit/dbc7a32f04fb282691f42452e5e765faaaa5068f |
| − | | | + | | https://klvincho.wordpress.com/2018/04/08/osd600-lab6/ |
|- | |- | ||
| 4 | | 4 | ||
| − | | | + | | Joseph Pham |
| − | | | + | | https://github.com/jpham14/browser-laptop/commit/628fe2914f5d20b3062db457249a5ff514d610d4 |
| − | | | + | | https://jpham14.wordpress.com/2018/04/09/lab-6-brave-browser-bug/ |
|- | |- | ||
| 5 | | 5 | ||
| − | | | + | | Owen Mak |
| − | | | + | | https://github.com/Owen-Mak/browser-laptop/commit/02ac6dc499b68cfc8e7864d6a641d1ca36bbc564 |
| − | | | + | | https://makowen.wordpress.com/2018/04/12/brave-url-testing/ |
|- | |- | ||
| 6 | | 6 | ||
| − | | | + | | Lucas Verbeke |
| − | | | + | | https://github.com/Micluc/browser-laptop/commit/75f6e90f17f5a8bb096d70b60618832d3061b1d5 |
| − | | | + | | https://thelucasexcerpt.wordpress.com/2018/04/12/osd600-lab-6-lucas-verbeke |
|- | |- | ||
| 7 | | 7 | ||
| − | | | + | | Hao Chen |
| − | | | + | | https://github.com/haoRchen/browser-laptop/commit/7508291fb97050caaf714e8de6b4224d6eb87e55 |
| − | | | + | | https://medium.com/haorc/tdd-in-brave-browser-6cdf5162729c |
|- | |- | ||
| 8 | | 8 | ||
| − | | | + | | Qiliang Chen |
| − | | | + | | https://github.com/KignorChan/browser-laptop/compare/translate-zh-CH...fix-url?diff=unified&name=fix-url |
| − | | | + | | https://qchen102.blogspot.ca/2018/04/osd600-lab-6-fixing-url-bug-in-brave.html |
|- | |- | ||
| 9 | | 9 | ||
| − | | | + | | Hongcheng Zhang |
| − | | | + | | https://github.com/brave/browser-laptop/compare/master...StevenZhang123:browser-laptop |
| − | | | + | | https://hongcheng1993.wordpress.com/2018/04/15/lab6/ |
|- | |- | ||
| 10 | | 10 | ||
| − | | | + | | Alex Wang |
| − | | | + | | https://github.com/AlexWang-16/browser-laptop/commit/64526635e6e39e1dbadbf915e0010f8465170285 |
| − | | | + | | https://alexopensource.wordpress.com/2018/04/05/fix-url-issues-with-brave-browser-using-test-driven-development/ |
|- | |- | ||
| 11 | | 11 | ||
| − | | | + | | Soutrik Barua |
| − | | | + | | https://github.com/buttersnipps/browser-laptop/commit/00f4a3fde46d4173ba1c06b90ce62536161951a3 |
| − | | | + | | http://soutrikbarua.blogspot.ca/2018/04/fixing-url-bug-in-brave.html |
|- | |- | ||
| 12 | | 12 | ||
| − | | | + | | Abdul Kabia |
| − | | | + | | https://github.com/AbdulKabia/browser-laptop/commit/28caa8563b86b122caa70970d9741cf8f5ca38cd |
| − | | | + | | https://akkabia.wordpress.com/2018/04/15/show-me-dog-cat-pictures/ |
|- | |- | ||
| 13 | | 13 | ||
| − | | | + | | Zukhruf Khan |
| − | | | + | | https://github.com/zeddkay/browser-laptop/commit/dbd2ddd869c5fc38ee44a25545f4ae153ed6dab8 |
| − | | | + | | https://zedsdps909blog.wordpress.com/2018/04/16/fixing-a-bug-adding-tests/ |
|- | |- | ||
| 14 | | 14 | ||
| − | | | + | | Vimal Raghubir |
| − | | | + | | https://github.com/Vimal-Raghubir/browser-laptop/commit/32cb08efbb5021e1dbdaa494d76c2d4370913a34 |
| − | | | + | | https://medium.com/@vraghubir/brave-browser-bug-fix-eae6ad5b9c23 |
|- | |- | ||
| 15 | | 15 | ||
| − | | | + | | Oleh Hodovaniuk |
| − | | | + | | https://github.com/ohodovaniuk1/browser-laptop/commit/2f41d18cd0b6defaf349d8c859447a1c668f3929 |
| − | | | + | | https://ohodovaniuk.wordpress.com/2018/04/07/osd600-lab6/ |
|- | |- | ||
| 16 | | 16 | ||
| − | | | + | | Patrick Godbout |
| − | | | + | | https://github.com/Owen-Mak/browser-laptop/commit/02ac6dc499b68cfc8e7864d6a641d1ca36bbc564 |
| − | | | + | | https://mb30myopensourceblog.blogspot.ca/2018/04/fixing-bug-adding-tests.html |
|- | |- | ||
| 17 | | 17 | ||
| − | | | + | |Pranoy Santosh |
| − | | | + | |https://github.com/pranoy10/browser-laptop/commit/1d8d4c08f06ab05d10b0d98e9b3b699e0934af5e |
| − | | | + | |http://pranoydps909.blogspot.ca/2018/04/brave-browser-bug-fix.html |
|- | |- | ||
| 18 | | 18 | ||
| − | | | + | |Kevin Pham |
| − | | | + | |https://github.com/pranoy10/browser-laptop/commit/1d8d4c08f06ab05d10b0d98e9b3b699e0934af5e |
| − | | | + | |http://kqpham2.blogspot.ca/2018/04/brave-browser.html |
|- | |- | ||
| 19 | | 19 | ||
| − | | | + | |Michael Fainshtein |
| − | | | + | |https://github.com/mfainshtein2/browser-laptop/tree/lab6 |
| − | | | + | |https://moderatelyokaydeveloper.wordpress.com/2018/04/21/file-url-we-need-some-space/ |
|- | |- | ||
| 20 | | 20 | ||
| − | | | + | | Woodson Delhia |
| − | | | + | | https://github.com/AlexWang-16/browser-laptop/commit/64526635e6e39e1dbadbf915e0010f8465170285 |
| − | | | + | | https://woodsondelhia.wordpress.com/2018/04/24/lab-6-brave-browser |
|- | |- | ||
| 21 | | 21 | ||
| − | | | + | | Justin Vuu |
| − | | | + | | https://github.com/jevuu/browser-laptop/commit/7bec15a0ca258c73d6877e569c72f4b6d143546a |
| − | | | + | | https://justosd.wordpress.com/2018/04/23/osd600-lab-6-fixing-a-bug-and-adding-tests/ |
|- | |- | ||
| 22 | | 22 | ||
| − | | | + | | Bakytzhan Apetov |
| − | | | + | | https://github.com/apetov/browser-laptop/commit/420bfa5076bfc0477dd006af2ddef7f1d56e4508 |
| − | | | + | | https://bapetov.wordpress.com/2018/04/23/lab-6-fixing-white-space-search/ |
|- | |- | ||
| 23 | | 23 | ||
| − | | | + | | Zhihao Cai |
| − | | | + | | https://github.com/josechoy/browser-laptop/commit/fa607bd5e8f3bb2cee4bc00dda58c83f68bdbff5 |
| − | | | + | | https://choyzhihao.wordpress.com/2018/04/23/tdd-practice-in-brave/ |
|- | |- | ||
| 24 | | 24 | ||
| − | | | + | | Aleksey Glazkov |
| − | | | + | | https://github.com/alexglazkov9/browser-laptop/commit/39d2db37cb403adc264abbdf01a820dd376da26b |
| − | | | + | | https://aglazkovblog.wordpress.com/2018/04/29/dps909-lab-6/ |
|- | |- | ||
| 25 | | 25 | ||
Latest revision as of 21:29, 28 April 2018
Contents
Fixing a Bug, Adding Tests
This week we're looking at the Brave browser. We spent time working through a walkthrough of a bug fix together. Now it's your turn to try and fix a bug and add more tests.
In this lab, you are asked to fix a bug in Brave using Test Driven Development (TDD). With TDD, we write tests before we implement the code, and work toward getting our tests to pass. The goals of the lab include:
- Gain experience working on another large open source project
- Learn how to write unit tests via TDD for a bug fix we hope to make
- Learn how to run individual tests in a big project
- Fix a bug in big code
- Read parallel implementations in other large open source projects (Firefox, Chrome)
- Read and Write code written in modern JavaScript
NOTE: you do not need to submit a Pull Request to Brave.
1. Get a Partner
While you're not required to do this lab with a partner, I encourage everyone who wants to do so to go ahead and find one. Open source is best when done in community, and learning how to work with others on debugging and code fixes is a good skill.
After today's lab, you can continue to work on Slack.
2. Build Brave
Begin by forking, cloning, and building Brave. Follow the appropriate instructions for your platform:
- https://github.com/brave/browser-laptop#running-from-source
- https://github.com/brave/browser-laptop/wiki/(setup)-Windows-build-guide
3. The Bug
Modern web browsers allow users to use the URL Bar in order to enter URLs, but also search terms that are meant to be passed to a default search engine. Sometimes a URL can look like a search term, or vice versa, and make it difficult to tell the browser what you want it to do.
For each of the cases listed below, compare the result in Chrome, Firefox, and Brave.
NOTE: I've put double-quotes around the text in order to show you where spaces exist. Remove the double-quotes, and try each one in your test browsers.
-
"dog" -
" dog " -
"dog cat" -
" dog cat " -
"https://www.google.ca/search?q=dog" -
" https://www.google.ca/search?q=dog " -
"https://www.google.ca/search?q=dog cat" -
" https://www.google.ca/search?q=dog cat "
For the next set of test cases, you need to create a file on disk with a space in its filename:
-
echo "hello" > "dog cat.txt"(macOS/Linux) -
Echo "hello" > "dog cat.txt"(Windows)
Take note of the absolute path to your file. On my macOS machine it's /Users/humphd/repos/browser-laptop/dog cat.txt, but it will be different for everyone, and quite different on Windows vs. macOS/Linux.
Now try using your absolute path in the URL bar (i.e., change my path below to yours, but keep the same structure):
-
"/Users/humphd/repos/browser-laptop/dog cat.txt" -
" /Users/humphd/repos/browser-laptop/dog cat.txt " -
"file:///Users/humphd/repos/browser-laptop/dog cat.txt" -
" file:///Users/humphd/repos/browser-laptop/dog cat.txt "
Record any cases where Brave did something unexpected, especially where it deviated from what Chrome and Firefox do.
4. Compare Implementations
Sometimes it's helpful to compare how different projects implement the same feature. You can often learn a lot by reading the code in a project that does what you want, in order to make another do new things.
We'll compare the code for Firefox, Chrome, and Brave:
- The code for fixing URL bar input in Brave is written in JavaScript, see https://github.com/brave/browser-laptop/blob/master/js/lib/urlutil.js
- The equivalent code in Firefox is written in C++, see https://searchfox.org/mozilla-central/source/docshell/base/nsDefaultURIFixup.cpp
- The equivalent code in Chrome is also written in C++, see https://cs.chromium.org/chromium/src/components/url_formatter/url_fixer.cc and https://cs.chromium.org/chromium/src/components/url_formatter/url_fixer.h
Read through both files and try to orient yourself. Don't get hung-up on understanding every single line, especially in the C++ code, which uses lots of internal string APIs, XPCOM code, etc. Instead, try to find clues as to how each one is doing what it does with input strings before converting them to URLs.
5. Add Test Cases
For each of the cases in 3. above where you found a problem, create a new unit test in Brave.
The unit tests for URL parsing in Brave are stored in https://github.com/brave/browser-laptop/blob/master/test/unit/lib/urlutilTest.js
Each describe() block defines a set of tests for a group of functionality in Brave's URL utility code. Find a logical place to add some new it() test functions. For example, look at describe('getUrlFromInput', function () {...}).
Read the existing tests in order to understand how to structure your own test cases. You can also read the mocha docs and assert docs.
6. Run the URL Tests
Tests are really code that needs to be run, and like all code in Brave, you'll have to "build" the test code before you can run the tests.
Follow the instructions at https://github.com/brave/browser-laptop/blob/master/docs/tests.md, specifically:
- install the testing framework
mocha:npm install --global mocha - build the tests with
npm run watch-test - run the unit tests with
npm run unittest - run a subset of the unit tests by adding a
grepexpression for the text in the describe/it string:npm run test -- --grep="getUrlFromInput"
Make sure that your new tests are being run. Because you haven't changed any code yet, your new tests probably fail, which is what we'd expect.
7. Get Your Tests to Pass
Now that we have found our failing tests cases, and written tests, it's time to try and fix the code. Modify the code in https://github.com/brave/browser-laptop/blob/master/js/lib/urlutil.js and re-run your tests. If something fails, go back and fix the code again, re-run the tests, and repeat.
Continue this process until you have fixed the code such that all the tests pass.
8. Submission
Create a branch and commit/push your changes to your fork as a single commit. Get a URL to your commit, which will look something like https://github.com/{your-username}/browser-laptop/commit/83f4763c482bbe0ba07a41711991beee709664bb.
Write a blog post about your experience doing the lab. Here are some things to write about:
- Which URL string cases did you have trouble with in Brave vs. other browsers?
- Did you notice any other interesting differences between browsers and how they handle strings in the URL bar?
- How did you write tests for these cases?
- What did you notice in the Firefox/Chrome code that was both similar and different to the Brave URL handling code?
- What did you have to do in order to get Brave to work the same as other browsers?
- What did you need to learn in order to complete the work?
Please add a line for your blog and GitHub commit in the following table:
