-
Notifications
You must be signed in to change notification settings - Fork 186
Z Intern Suggestions (Sum 2017)
(now restructuring this page in backwards chronological order; most recent topics at top)
- consider the use of a coding styleguide (either mozilla's or one for webcompat.com). this would save time in the review process.
- git shortcuts that can be added to unix scripts
-
when adding functional tests, want to avoid any test that saves an issue to github (otherwise might get complaint of spam from github)
-
mozilla coding style guide
- issue 1297 is pending a response
sort of rough thoughts on high level restructure of docs on testing (webcompat.com)
prerequisites
- java (jdk vers); think docs needs a link update for mac os
running tests (typical developer workflow - maybe more approp somewhere else)
- make code change
- do some manual tests (if creating/saving issues manually run server: python run.py)
- commit changes
- run all functional tests (run server: python run.py -t)
- somewhere note if you're seeing about 10 failing tests with issues and issue lists, probably you are not running the server with -t param
- node_modules/.bin/intern-runner config=tests/intern functionalSuites=tests/functional-all user='[github_user]' pw='[github_password]'
- run pep8 (for python)
- run npm run lint (for js)
- if all tests passing, rebase commits
- push to your forked repo
- make a pr
tips for running tests quickly
- sometimes a contributor might need to rerun an individual test multiple times (while debugging a new test or fixing an existing test that new code broke). for these scenarios it is painful to have to run full test suites (particularly with the additional browser test, although it is a really cool test feature addition <3).
- in the future, it would be nice to have better documentation for the general features being tested in each test file (for clarity where to add new tests)
- in process of setting up new (mozilla) laptop for webcompat.com contributions
- for running tests, java is required; safari browser will default a link to the java jre version but I found this problematic to configure for testing (couldn't tell how to point to appropriate /bin folder); I think even the contributor docs is defaulting to the jre installation
- worked around by installing java sdk version instead (it now appears to be set up by default with links so you do not even need to update PATH env var); also in my previous dev env, I had java sdk installed and it seems easier to work with for running tests
- when testing webcompat.com in local build, if unable to save new issues it is probably an issue with /config/secrets.py file > ISSUES_REPO_URI values (see 2 places)
We use Grunt as a task runner to perform certain tasks (minify + concat JS assets, for example). You need to have Node.js installed to be able to run Grunt. Once that's done, npm can be used to install Grunt and other build dependencies.
? maybe give the command, like npm install -g grunt
(in case it may not be obvious; or if local install is better, update package.json?)
- notes based on https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Mac_OS_X_Prerequisites
- had to tweak the steps before able to run
python bootstrap.py - install openssl using homebrew
- install python (2.7.*) using homebrew
- here since python was originally installed first, I had to overwrite the existing installation (force overwriting the link)
- turned out that bootstrap.py could be run with python 2.7.13
- notes for revised required fields validation flow: https://github.com/webcompat/webcompat.com/pull/1633#issuecomment-312531153
- this is probably something to be considered for next gen app (when moving to another js framework)
(quick placeholder to capture my thoughts but need to flesh out the idea in the future; perhaps the idea is off b/c of my short perspective as an outsider)
- through catching snippets of history of the group, I gather that one of the intents of webcompat.com service is public service
- part of me feels like the receiving companies (Webcompat bug fix recipients) should somehow pay back the webcompat group or in the least offer to buy a cup of very nice coffee in return; however I feel perhaps it is not in the spirit of the project to get recompense for the work
- one challenge might be for the Webcompat group to promote its value back to the larger Mozilla org. if it is not done currently perhaps it is possible to get some metrics as to the use of firefox for sites who have received Webcompat fixes (before and after? not 100% sure what is appropriate to measure)
- while reviewing the code ran across a little bit of cursing in a code comment. I strongly feel it would be better not to have cursing in the code because it creates a slight tone of hostility but more importantly, it shows an attitude of not caring
- action item: review the contributing doc to see if there is a mention about language
- action item: update the source code file (this can be part of a larger refactoring review)
- it would be nice to add a label for Softvision folks (QA) to indicate when an issue has been fixed/merged and it ready to retest/close. in the past I have used "Please Retest" but this could be edited.
- while doing research for #1565, noted there is a lot of redundant JS code for views like index.html (/), /issues, and /activity views. this seems to be a result of rendering the issues list (with varying filters) from at least 3 views.
- although this is under @zoepage's area, I would suggest for next gen application, there should only be one view for the issues list and a thorough set of filters available (triage, existing labels, my activity - created by user)
- this should help keep the code more dry and easier to maintain
- additionally I do not feel there is that much benefit to the user to have the issues list from 3 different views; it provides clarity to have them linked in one place
- this is a nice view to skim activity across the group: https://github.com/orgs/webcompat/dashboard
Onboarding: maybe it would be nice to have quarterly (or needs based timing) video hangouts (30min) for Q&A for new contributors. this would allow for 2 things:
- information exchange (staff could also understand what is not clear from the website)
- a little better sense of human engagement (I feel this can go a long way towards helping people feel belonging to a project)
- background rationale: as a newcomer, the irc format is very intimidating for understanding who does what and who to ask what...
- maybe consider to do bi yearly video hangouts for Q&A for other people at Mozilla. during the reception, I chatted with someone in another group and they were sincerely curious about what Webcompat did and were not very aware of the project/group. it appears easy with the company size to get silo'd.
(assumes pushing branches to a forked repo and upstream points to master on webcompat/webcompat.com)
How to sync with webcompat/webcompat.com master if you get out of sync
git checkout -b backup master
git fetch upstream
git branch -D master
git checkout -b master upstream/master
(notes from karlcow)
How to update node modules (assumes you've already run npm install at some previous point in time but package.json was updated afterward for a node module; ex some dependency was upgraded)
rm -rf node_modules
(note from miketaylr)
How to squash commits (before making a PR; modifies pr-coding-guidelines.md)
(from Mozilla WebQA)
How to rebase branch (before making a PR for contributors; modifies pr-coding-guidelines.md)
it would be very nice to have a complete pr prerequisite checklist of todos documented for contributors
- (new 070217) I've contributed to a different project where they automated some github process when you make a pr for their repo, the resulting github screenshot (very first comment box for pr) is prepopulated with a checklist of prerequisites for a successful pr (ie linting, squash commits, rebase, correct pr title format, etc); this sort of thing might be useful to copy...
Running local tests for contributors
Might want to suggest that the fastest way to run the majority of tests (non auth) is by:
node_modules/.bin/intern-runner config=tests/intern functionalSuites=tests/functional-nonauth
Recommend not to run cpu or memory intensive processes simultaneously with functional tests (ex streaming audio or video is probably going to result in more test failures than normal)
Misc Tips
-
How to iterate over multiple elements using leadfoot(intern) selectors
-
(new) To verify that intern/leadfoot selectors are accurate, it can be helpful to write a purposefully failing test . Then the assertion can compare the value (for ex, getVisibleText()) for the target selector against something intended to fail, like ''. This way, the report from the test will print out the text value from the selector and help debug the selector.
- (new) some README/docs topics are relevant for new contributors but there are a few topics that are specific to new webcompat repo members; some instructions should be different depending on repo permissions
- in which case would a contributor want to run their own nginx server?
- some architecture diagram to understand data flow through the system
- some explanation of how/where the data gets stored (before I got confused and created an issue on prod for testing so this understanding would clarify why someone should not do that)
- towards the end of the summer, it would be nice to have some list of expected test failures (maybe this would be resolved at that point) b/c as a contributor it feels very confusing to make a PR where there are a bunch of local test and travis test failures that you cannot explain and are not sure whether your code change broke previously passing tests
- probably should go through a js code refactor before the process of moving from backbone to a new js framework happens (ie remove obsolete functions, variables; simplify if possible)
- maybe python refactor too, although much of it looks pretty clean already