feat: Added --watch-ignored new command line option

[ankushduacodes]

fixes #2022

[coveralls]

Coverage remained the same at 100.0% when pulling 131d700 on ankushduacodes:addin_watch-ignored_functionality into 84886ba on mozilla:master.

[ankushduacodes]

@rpl, I am not sure why onChange function is never being called even if we write to a file that is not being ignored.
After doing some debugging, I found that watcher.on('change') event is never being invoked

web-ext/src/watcher.js

Lines 50 to 52 in afd9592

	watcher.on('change', (filePath) => {
	proxyFileChanges({artifactsDir, onChange, filePath, shouldWatchFile});
	});

Doubt:

1. What difference does it make when we use watchChange helper function instead of using onSourceChange directly?, why does it do intial scan when we use watchChange?

Thank you!...

Update: I see that tests on travis-ci are passing successfully but on my machine --watch-ignored test is failing. Here's a screenshot of that:

On linux, tests are passing successfully

[ankushduacodes]

@rpl I think I have added all the necessary test, my only two doubt/update are outlined in this comment. Hoping to understand them better :)

Do let me know any other changes are required.

(PS: I am still new to JavaScript and always trying to better myself. If you have any beginner-friendly issue, I would love to take part in it. Thank you for the opportunity!.. 😊)

[rpl]

My apologies again for the delay. Follows a new round of review comments (some nits, and a bigger and more detailed comment related that tricky test case ;-)).

[ankushduacodes]

@rpl, I have made the necessary changes outlined here.
The tests are passing on both macOS and Linux.
I have also resolved other requested changes.

Please let me know how it looks.
Thank you.

Edit: Could I please also ask how was race condition happening earlier?

[rpl]

@ankushduacodes looks great, follows just two more nits (one small tweak to the option description and moving one of the new test case into the existing group of watcher tests).

In the following comments there is one that provides some more details about the possible reasons for the intermittent failures (and some details about a small stress test experiment I did locally to double-check if that test can still fail from time to time as I was guessing, well... spoiler... it still can :-P, but I'm ok on deferring looking more into that in a follow up if the intermittent failures are going to actually happen after we landed this on master).

[ankushduacodes]

@rpl I am not sure why Circle CI is giving me a build error. Is this internal to web-ext itself?

[rpl]

@rpl I am not sure why Circle CI is giving me a build error. Is this internal to web-ext itself?

@ankushduacodes we migrated yesterday to CircleCI, would you mind to rebase this branch on top of the last changes landed in the repo? that would make the circleci config available in this pull request and CircleCI should stop complaining and start running the 4 CI jobs expected (e.g. like the list of circleci checks in PR #2095).

---

Some more context in case you are interested in knowing more:

We had to migrate the CI jobs to CircleCI asap to close #2094 before the end of the year and be sure to keep our CI jobs up and running. I landed the changes yesterday on master (#2096) and so now circleci complains because in this branch there is no CircleCI config (the log in the CI failure is No configuration was found in your project.) and apparently Travis isn't building neither (I guess that's because the master branch does not have the travis config file anymore, removed in #2096 along with replacing it with the CircleCI config).

[rpl]

@ankushduacodes Thanks for rebasing the PR right away (and for covering all the last review comments in the updated version of this patch), you are awesome ❤️

I'm approving this version, but I'll wait your feedback about that experiment with the stress test (the one related to double-checking that new test case for intermittency issues) before proceeding to merge it.

Ping me in a comment to let me know when you have done with that (either if there are further changes applied to this PR or not).

Thanks again for your work on these contributions!

[ankushduacodes]

@rpl According to me the only thing left to do in this PR is an update to the docs. What do you think?

[rpl]

@rpl According to me the only thing left to do in this PR is an update to the docs. What do you think?

yep, I'm going to give a quick look to the last update and then merge it on master.
After that, the change has to be documented (in the extension-workshop doc page) and released on npm.

We are not planning to release a new web-ext version before the end of the year and so there is no rush to create the pull request for the mozilla/extension-workshop, we would still merge that extension-workshop PR right after the new option has been released as part of a new web-ext version, but feel free to open it and mention it in a new comment on #2022.

Thanks again for you work on this enhacement! (and for the help investigating the test intermittency on macos! In my opinion that turned out to be a pretty interesting part of this PR ;-)).