fix: Can't run with specified profile name and --keep-profile-changes #968

harikishen · 2017-06-08T17:22:41Z

Fixes #932

coveralls · 2017-06-08T17:25:34Z

Coverage decreased (-1.09%) to 98.908% when pulling 9b7ce5a on harikishen:ticket_932 into 50528bf on mozilla:master.

harikishen · 2017-06-08T19:14:07Z

the failure seems to be because of the title of the PR.

rpl · 2017-06-12T10:48:23Z

Hi @harikishen
First of all, my apologies for the time I needed to be able to look into this PR.

Currently the tests are only failing on the PR title linting, nevertheless, most of the code from this PR is currently not covered by the tests (in other words: "it is currectly not tested"), as you can see from the coveralls failures and related reports, e.g.:

https://coveralls.io/builds/11889712/source?filename=src%2Ffirefox%2Findex.js#L259

By looking in a bit more details into the diff, the first thing that I notice is that you are currently trying to use finder.getPath('default') as a function that synchronously return a result, while it is actually a function that call a callback asynchronously and pass the result as a parameter to it.

To make the code more readable I have probably already suggested you, in our last chat on IRC about this change, to use es6-promisify (which is already in the web-ext dependencies and we are currently using it to wrap the zip-dir library methods with promise-based wrapper functions), but it looks that we are not yet wrapping the finder object using es6-promisify (and we should also use await on these calls to make the async function able to wait the returned promises to be resolved).

To learn more about how to use es6-promisify, you can look at the project README here:

https://github.com/digitaldesignlabs/es6-promisify

To summarize the changes that are still needed on this PR to put it on the right direction (at least from my current point of view):

using es6-promisfy to wrap the finder object methods
using await to wait a promise to be resolved or rejected
defining additional tests to cover the new scenarios (and the new lines of code) introduced by this PR

coveralls · 2017-06-14T13:56:52Z

Coverage decreased (-31.2%) to 68.758% when pulling 2be9919 on harikishen:ticket_932 into 50528bf on mozilla:master.

coveralls · 2017-06-14T16:03:25Z

Coverage decreased (-30.6%) to 69.417% when pulling 806f9e0 on harikishen:ticket_932 into 50528bf on mozilla:master.

rpl · 2017-06-15T13:08:28Z

src/firefox/index.js

@@ -251,7 +251,37 @@ export async function useProfile(
    customPrefs = {},
  }: UseProfileParams = {},
 ): Promise<FirefoxProfile> {
-  const profile = new FirefoxProfile({destinationDirectory: profilePath});
+  let profile;
+  const finder = new FirefoxProfile.Finder();


we will need to be able to customize the path where the profiles will be searched by the profile Finder helper class, to be able to more easily test these behaviors in the unit tests.

And so this function should take an additional searchProfilesPath (or another similar identifier name) parameter which defaults to undefined (so that the default behavior of the Finder class will be preserved when this method is executed without passing the search profile path explicitly).

rpl · 2017-06-15T13:19:32Z

src/firefox/index.js

-  const profile = new FirefoxProfile({destinationDirectory: profilePath});
+  let profile;
+  const finder = new FirefoxProfile.Finder();
+  const finderGetPath = promisify(finder.getPath.bind(finder));


finderGetPath will reject with an Error when the Finder instance is not able to find the requested profile name, which is something that we should expect to happen (e.g. because the user could have no profile with that name yet).

And so, to be able to only call findGetPath when we know that it will resolve to the path instead of being rejected with an Error, we can use the finder.readProfiles method and check if the named profiles that we need actually exists before trying to resolve them into paths, e.g. something like the following snippet should do the trick:

... const finder = new FirefoxProfile.Finder(searchProfilesPath); const finderGetPath = promisify(finder.getPath, finder); const finderReadProfiles = promisify(finder.readProfiles, finder); // Read the profiles list from the profiles.ini file from `searchProfilesPath`. await finderReadProfiles(); // Helper function which returns true if the profile name exists in the profiles.ini file. const hasProfileName = (profileName) => { return finder.profiles.filter( (profileDef) => profileDef.Name === profileName ).length !== 0; }; ... const defaultProfilePath = ( hasProfileName('default') && await finderGetPath('default') ); const defaultDevProfilePath = ( hasProfileName('default') && await finderGetPath('dev-edition-default') );

This way defaultProfilePath and defaultDevProfilePath will be the profile path (which is a string), or undefined if the profile doesn't exist in the profiles.ini list.

rpl · 2017-06-15T13:20:50Z

src/firefox/index.js

+    if (dirExists) {
+      log.debug(`Copying profile directory from "${profilePath}"`);
+      const defaultProfilePath = await finderGetPath('default');
+      const defaultDevProfilePath = await finderGetPath('default-dev-edition');


The default name of the Firefox DevEdition profile is 'dev-edition-default' (instead of 'default-dev-edition')

rpl

Hi @harikishen
Here is some additional feedback (mostly related to the usage of the Finder class).

I just took a deeper look into the Finder class implementation from the "firefox-profile" dependency and I added above some inline feedback comments about how we can check upfront if the default profiles names exists (before trying to resolve the profile name into profile paths using finder.getPath).

harikishen · 2017-06-15T20:17:24Z

@rpl could you look into it now ?

harikishen · 2017-06-15T20:17:59Z

i've written 1 more test for named profiles.

coveralls · 2017-06-15T20:22:07Z

Coverage decreased (-0.7%) to 99.275% when pulling 854b09c on harikishen:ticket_932 into 50528bf on mozilla:master.

coveralls · 2017-06-16T06:48:14Z

Coverage decreased (-26.0%) to 74.034% when pulling 5876d14 on harikishen:ticket_932 into 50528bf on mozilla:master.

coveralls · 2017-06-16T07:05:14Z

Coverage decreased (-0.6%) to 99.395% when pulling 9fc5e9d on harikishen:ticket_932 into 50528bf on mozilla:master.

harikishen · 2017-06-16T07:51:27Z

@rpl coveralls fails because of hasProfileName(). How do I write a test for it ?

rpl · 2017-06-16T11:53:16Z

src/firefox/index.js

+  const finder = new FirefoxProfile.Finder(searchProfilesPath);
+  const finderGetPath = promisify(finder.getPath, finder);
+  const hasProfileName = (profileName) => {
+    return finder.profiles.filter(


This part was not currently covered in the tests because finder.profiles is going to be an empty array if finder.readProfiles has not been called first.

rpl · 2017-06-16T12:05:46Z

src/firefox/index.js

  }: UseProfileParams = {},
 ): Promise<FirefoxProfile> {
-  const profile = new FirefoxProfile({destinationDirectory: profilePath});
+  let profile;
+  const finder = new FirefoxProfile.Finder(searchProfilesPath);


Given how we are using and testing the Finder class, I'm wondering if it would be better to create an createProfileFinder helper which creates the profile finder and wraps the methods that we need with promisify, something like

function defaultCreateProfileFinder() { const finder = new FirefoxProfile.Finder(); const readProfiles = promisify(finder.readProfiles, finder); return { getPath: promisify(finder.getPath, finder), hasProfileName: async () => { await finder.readProfiles(); return finder.profiles.filter( (profileDef) => profileDef.Name === profileName ).length !== 0; } } }

So that we can simplify useProfile:

export async function useProfile( profilePath: string, { ... createProfileFinder: defaultCreateProfileFinder, }: UseProfileParams = P{, ): Promise<FirefoxProfile> { const finder = createProfileFinder(); ... }

and more easily test its behavior, e.g. because we can override createProfileFinder with a function that returns a "fake" version of our profile finder wrapper (and so we dont' even need to create the real "profiles.ini" file or the fake profiles directory in tmpDir).

rpl · 2017-06-16T12:08:03Z

src/firefox/index.js

+        await finderGetPath('dev-edition-default')
+    );
+    if (dirExists) {
+      log.debug(`Copying profile directory from "${profilePath}"`);


This message does not seem right here, useProfile is never going to copy a profile.

rpl · 2017-06-16T12:10:03Z

src/firefox/index.js

+      }
+      profile = new FirefoxProfile({destinationDirectory: profilePath});
+    } else {
+      log.debug(`Assuming ${profilePath} is a named profile`);


I'm wondering if we should make this to always fail if the requested profile name is default or dev-edition-default.

@rpl what do you propose ?

something like:

if (profilePath === 'default' || profilePath === 'dev-edition-default') { throw new WebExtError(`Cannot use the blacklisted named profile "${profilePath}" `); }

rpl · 2017-06-16T12:12:22Z

src/firefox/index.js

+    }
+  } catch (error) {
+    throw new WebExtError(
+      `Could not copy Firefox profile from ${profilePath}: ${error}`);


As the message logged at line 275, this error message is also not the right one.

coveralls · 2017-06-16T14:38:57Z

Coverage decreased (-1.3%) to 98.678% when pulling 8c333de on harikishen:ticket_932 into 50528bf on mozilla:master.

rpl · 2017-06-19T19:34:46Z

tests/unit/test-firefox/test.firefox.js

+            hasProfileName: () => Promise.resolve(true),
+          };
+        };
+        const spy = sinon.spy(profileFinder, "getPath");


@harikishen this is probably a leftover, but it is making the CI to fail on the eslint checks and it never reaches the unit tests:

https://travis-ci.org/mozilla/web-ext/builds/244601162#L1351-L1353

rpl · 2017-06-19T19:39:47Z

tests/unit/test-firefox/test.firefox.js

+          sinon.spy((profile) => Promise.resolve(profile));
+        const app = 'fennec';
+        const profilePath = tmpDir.path();
+        const profileFinder = () => {


@harikishen profileFinder is a function that creates the fake Finder objects, and not a Finder object itself, that is the reason why the assertion commented on line 351 is not working for you.

You should do something like:

const profileFinder = { getPath: sinon.spy(...), ... }; const createProfileFinder = () => profileFinder; const profile = await firefox.useProfile(profilePath, { ... configureThisProfile: configureProfile, createProfileFinder: createProfileFinder, }); assert.equal(configureProfile.called, true); ... assert.equal(profileFinder.getPath.callCount, 2)

harikishen · 2017-06-19T19:54:23Z

@rpl what do you think of the changes in index.js ?

rpl · 2017-06-19T20:08:18Z

src/firefox/index.js

+    hasProfileName: async (profileName: string) => {
+      try {
+        await fs.stat(path.join(FirefoxProfile.Finder.locateUserDirectory(),
+                                                              'profile.ini'));


"profiles.ini"

Actually I was trying the case if the file did not exist... forgot to fix it

rpl · 2017-06-19T20:12:26Z

src/firefox/index.js

+                                                              'profile.ini'));
+      } catch (error) {
+        if (isErrorWithCode('ENOENT', error)) {
+          log.info('No firefox profiles exist');


Here, we have already checked that "profiles.ini" doesn't exist (which should probably be a warning instead of an info message), but we are going to execute await readProfiles() at line 266, which will raises an exception because of that.

And so I think that it would be reasonable to return false after logging a warning (no profile name can be found if no "profiles.ini" has been found).

What is the call for warning...???

unsurprisingly log.warn :-) (

web-ext/tests/unit/helpers.js

Line 174 in 97404a9

log.warn(`Running stubbed function ${key} (default implementation)`);

)

harikishen · 2017-06-20T08:28:36Z

@rpl i've done the requested changes. What else remains to be done ?

coveralls · 2017-06-20T08:32:47Z

Coverage decreased (-1.4%) to 98.568% when pulling ac9a6b2 on harikishen:ticket_932 into 50528bf on mozilla:master.

rpl · 2017-06-20T13:52:23Z

tests/unit/test-firefox/test.firefox.js

          sinon.spy((profile) => Promise.resolve(profile));
+        const profileFinder = () => {


it would be nicer if we can keep the same naming convention in all the tests, e.g. this profileFinder identifier is the function that creates the fake Finder instance in some of the tests and the fake Finder instance in other tests.

Also, if you name these identifier like the params property names, you can use the ES6 short syntax, eg.

const createProfileFinder = () => ...; const configureThisProfile = () => ...; ... return firefox.useProfile(profilePath, {app, configureThisProfile, createProfileFinder});

rpl · 2017-06-20T13:53:49Z

tests/unit/test-firefox/test.firefox.js

        const app = 'fennec';
        const profilePath = baseProfile.path();
-        return firefox.useProfile(profilePath, {app, configureThisProfile})
+        return firefox.useProfile(profilePath,


most of these tests would be easier to read if we use the await syntax here

const profile = await firefox.useProfile(...); ... // test assertions

rpl · 2017-06-20T13:57:00Z

src/firefox/index.js

 };

+export function defaultCreateProfileFinder() {


All the new tests are using a fake createProfileFinder, and so (like the code coverage reports confirm) it is time to create some tests specifically for defaultCreateProfileFinder).

To make this function more easily testable we should add an optional userDirectoryPath (a string) parameter to it, so that we can customize the directory where the Finder class will search the profiles.ini file and the profile directories.

what tests do you propose ?

rpl · 2017-06-20T13:58:39Z

src/firefox/index.js

+      log.debug(`Using profile directory from "${profilePath}"`);
+      if (profilePath === defaultProfilePath ||
+        profilePath === defaultDevProfilePath) {
+        throw new WebExtError(


By looking at the coverage reports, this line doesn't seem to be currently executed by any of the current tests.

coveralls · 2017-06-20T15:48:58Z

Coverage decreased (-1.3%) to 98.687% when pulling 232e1e5 on harikishen:ticket_932 into 50528bf on mozilla:master.

harikishen · 2017-06-20T17:35:33Z

@rpl The issue seems to be with some sort of race condition with fs.stat() call and the await readProfiles() call. Correct me if I'm wrong.

coveralls · 2017-06-20T17:38:22Z

Coverage decreased (-0.4%) to 99.642% when pulling fdff679 on harikishen:ticket_932 into 50528bf on mozilla:master.

rpl · 2017-06-22T12:12:00Z

src/firefox/index.js

+    getPath: promisify(finder.getPath, finder),
+    hasProfileName: async (profileName: string) => {
+      try {
+        await fs.stat(path.join(FirefoxProfile.Finder.locateUserDirectory(),


@harikishen the tests are failing because on an issue in the hasProfileName implementation:

while we are configuring the Finder to use the specified parameter as the profiles search path (as expected)
at line 251, here we are always doing an fs.stat on the default search path the FirefoxProfile.Finder.locateUserDirectory() provides (but it should use the same path that the Finder is going to use

And so it should be something like:

const profilesIniPath = path.join( userDirectoryPath || FirefoxProfile.Finder.locateUserDirectory(), 'profiles.ini' ); const res = await fs.stat(profilesIniPath);

rpl · 2017-06-22T12:13:04Z

src/firefox/index.js

 };

+export function defaultCreateProfileFinder(userDirectoryPath: string = '') {


this should be ?string (a string that can be null) and it should not default to an empty string
(it should be undefined when not specified, and so we can just remove the explicit assigned default)

rpl · 2017-06-22T12:24:17Z

src/firefox/index.js

@@ -99,6 +99,11 @@ export interface FirefoxProcess extends events$EventEmitter {
  kill: Function;
 }

+export interface IProfileFinder {


This flow type is currently unused and it should be used for the defaultCreateProfileFinder return value type annotation (we should also move it near that function, that is where it is going to be used).

Also, the type signatures are wrong: both hasProfileName and getPath take a string as parameter
(and so they should be hasProfileName(string): Promise<boolean>, and getPath needs the same kind of fix).

rpl · 2017-06-22T12:31:41Z

src/firefox/index.js

 };

+export function defaultCreateProfileFinder(userDirectoryPath: string = '') {


This defaultCreateProfileFinder function should have a type annotation of the return value:

export function defaultCreateProfileFinder(userDirectoryPath: ?string): IProfileFinder { ... }

Once the return value is "type annotated" correctly, flow will notice some new errors that needs to be fixed, in particular:

hasProfileName needs the type annotation of the return value

hasProfileName: async (profileName: string): Promise<boolean> => { ... },

we should only catch the exception of fs.stat in the try...catch at line 256.

we should re-throw any exception that as been caught by the above try...catch which do not verify the isErrorWithCode('ENOENT', error) check

in the test file "test.firefox.js" the fake getPath defined at lines 417 and 444 should resolve to a string value:

diff --git a/tests/unit/test-firefox/test.firefox.js b/tests/unit/test-firefox/test.firefox.js index da6a612..caa54b8 100644 --- a/tests/unit/test-firefox/test.firefox.js +++ b/tests/unit/test-firefox/test.firefox.js @@ -414,7 +414,7 @@ describe('firefox', () => { sinon.spy((profile) => Promise.resolve(profile)); const createProfileFinder = () => { return { - getPath: () => Promise.resolve(), + getPath: () => Promise.resolve('/fake/profilePath'), hasProfileName: () => Promise.resolve(true), }; }; @@ -441,7 +441,7 @@ describe('firefox', () => { sinon.spy((profile) => Promise.resolve(profile)); const createProfileFinder = () => { return { - getPath: () => Promise.resolve(), + getPath: () => Promise.resolve('/fake/profilePath'), hasProfileName: () => Promise.resolve(true), }; };

coveralls · 2017-06-22T14:33:51Z

Coverage remained the same at 100.0% when pulling bfb1703 on harikishen:ticket_932 into 50528bf on mozilla:master.

harikishen · 2017-06-22T15:07:07Z

@kumar303 @rpl Could you guys check this out when your free ?

rpl

Hi @harikishen
Follows a new round of review comments (mostly tweaks to simplify and clean up the patch a bit, and some fixes around the exceptions handling).

rpl · 2017-06-23T16:55:52Z

src/firefox/index.js

+          `Cannot use profile at "${profilePath}"`
+        );
+      }
+      profile = new FirefoxProfile({destinationDirectory: profilePath});


we can make this function to call const profile = new FirefoxProfile(...) in a single place by limiting this block and the one that contains line 317 to only "resolve the final destinationDirectory value", and then creating the FirefoxProfile instance at the end of this function.

rpl · 2017-06-23T16:57:11Z

src/firefox/index.js

 };

+export function defaultCreateProfileFinder(userDirectoryPath?: string) {


The IProfileFinder flow type defined at line 102 is still unused, it should be used as the return value of this function
(and we should also apply the related fixes suggested in my previous round of comments, but first we should take a look at the changes that I'm suggesting in one of the comments below, they can be helpful to simplify this helper as well as the useProfile implementation before fixing this flow type checking related issue).

rpl · 2017-06-23T17:04:19Z

src/firefox/index.js

+        return finder.profiles.filter(
+          (profileDef) => profileDef.Name === profileName).length !== 0;
+      } catch (error) {
+        if (isErrorWithCode('ENOENT', error)) {


This try catch should only surround the await fs.stat call and it should re-throw any error that does not verify isErrorWithCode('ENOENT', error) (or, on the contrary, this function is going to return undefined on all the other kind of exception that can be caught from this try...catch).

rpl · 2017-06-23T17:14:03Z

src/firefox/index.js

+  try {
+    const dirExists = await isDirectory(profilePath);
+    if (await finder.hasProfileName('default')) {
+      defaultProfilePath = await finder.getPath('default');


As an alternative approach to this two if (await finder.hasProfileName(...)) { ... = await finder.getPath(...); } we can opt to return from createProfileFinder a single function that returns a string (the profile path) if there is such a profile name and undefined (or null) otherwise
(in other words: we can merge the hasProfileName and getPath function defined at line 253 into a single helper function with the behavior described above).

This change can make this method implementation cleaner and simpler, turning it into something like:

const getProfilePath = createProfileFinder(); const dirExists = await isDirectory(profilePath); let destinationDirectory; if (dirExists) { ... if (profilePath === getProfilePath('default') || profilePath === getProfilePath('dev-edition-default')) { throw new WebExtError(...); } destinationDirectory = profilePath; } else { ... if (profilePath === 'default' || profilePath === 'dev-edition-default') { throw new WebExtError(...); } destinationDirectory = getProfilePath(profilePath); if (!destinationDirectory) { throw new UserError( `The request "${profilePath}" profile name cannot be resolved to a profile path` ); } } .... const profile = new FirefoxProfile({destinationDirectory}); return await configureThisProfile(profile, {app, customPrefs});

rpl · 2017-06-23T17:17:55Z

src/firefox/index.js

+      const profileDirectory = await finder.getPath(profilePath);
+      profile = new FirefoxProfile({destinationDirectory: profileDirectory});
+    }
+  } catch (error) {


I'm not sure that this giant try...catch is actually useful, at least by looking currently we can just leave any other exception to be propagated to the callers.

harikishen · 2017-06-24T06:50:39Z

@rpl I've done some of the changes mentioned. Please look into it and tell me whether i'm on the right track.

harikishen · 2017-06-24T06:52:06Z

@rpl I have a lot of doubts regarding rewriting the tests. So I've commented them for now.

coveralls · 2017-06-24T06:53:40Z

Coverage decreased (-2.0%) to 98.013% when pulling 4c4d407 on harikishen:ticket_932 into 50528bf on mozilla:master.

rpl · 2017-06-30T20:14:57Z

@rpl I have a lot of doubts regarding rewriting the tests. So I've commented them for now.

Hi @harikishen, sorry for the waiting time (I've been traveling for a conference),
Can you please remove the comments added on these tests?

Commenting the tests that fail is worst than let them fail on travis, by leaving them to fail on travis I would be able to look at the failures and help you to figure out what to do about the failing tests.

In general, the main point of writing tests is not to make them pass, on the contrary, it is to see them fail when something has been changed and needs more changes to be handled correctly, so the main point of writing tests is to read the failures logs ;-)

rpl · 2018-04-27T10:21:21Z

I'm closing this PR because we extracted and merged part of it in #1007 (the part that fixes #1005) and the remaining pieces have been included in #1149 (which is not yet merged, but it doesn't have conflicts and it is nearer to be completed).

harikishen added 3 commits June 8, 2017 21:05

Modified useProfile()

e7712a0

Modified useProfile()

0af91f6

Fix mozilla#932 - Modified useProfile() to check for profile paths

9b7ce5a

muffinresearch added the state: pull request ready label Jun 8, 2017

harikishen changed the title ~~Fix #932 - Modified useProfile() to check for profile paths~~ fix: Can't run with specified profile name and --keep-profile-changes Jun 8, 2017

harikishen added 2 commits June 14, 2017 17:34

Promisified finder.getPath()

939a2a3

Promisified finder.getPath()

2be9919

Moved code from util/finder.js to firefox/index.js

806f9e0

rpl reviewed Jun 15, 2017

View reviewed changes

Added tests for useProfile()

854b09c

Added test for named profile

5876d14

Removed readProfiles() call

9fc5e9d

rpl reviewed Jun 16, 2017

View reviewed changes

Changes to finder

8c333de

Changes to finder

c09b22e

rpl reviewed Jun 19, 2017

View reviewed changes

Fixed tests

ac9a6b2

rpl reviewed Jun 20, 2017

View reviewed changes

Refactored tests for useProfile()

232e1e5

Adding tests for defaultCreateProfileFinder()

fdff679

rpl reviewed Jun 22, 2017

View reviewed changes

Added tests for defaultCreateProfileFinder()

bfb1703

rpl reviewed Jun 23, 2017

View reviewed changes

Modified to combine hasProfileName and getPath

4c4d407

shubheksha mentioned this pull request Jun 29, 2017

Can't run with specified profile name and --keep-profile-changes #932

Open

rpl mentioned this pull request Jul 17, 2017

fix: Prevent --keep-profile-changes on default Firefox profiles #1007

Merged

2 tasks

saintsebastian mentioned this pull request Dec 10, 2017

feat: support of named profiles #1149

Closed

rpl closed this Apr 27, 2018

rpl mentioned this pull request Apr 27, 2018

Using a default profile with --keep-profile-changes will break Firefox #1005

Closed

		sinon.spy((profile) => Promise.resolve(profile));
		const profileFinder = () => {

		};

		export function defaultCreateProfileFinder(userDirectoryPath: string = '') {

fix: Can't run with specified profile name and --keep-profile-changes #968

fix: Can't run with specified profile name and --keep-profile-changes #968

Conversation

harikishen commented Jun 8, 2017 • edited by kumar303 Loading

coveralls commented Jun 8, 2017

harikishen commented Jun 8, 2017

rpl commented Jun 12, 2017

coveralls commented Jun 14, 2017

coveralls commented Jun 14, 2017

rpl Jun 15, 2017 • edited Loading

Choose a reason for hiding this comment

rpl Jun 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpl left a comment • edited Loading

Choose a reason for hiding this comment

harikishen commented Jun 15, 2017

harikishen commented Jun 15, 2017 • edited Loading

coveralls commented Jun 15, 2017

coveralls commented Jun 16, 2017

coveralls commented Jun 16, 2017

harikishen commented Jun 16, 2017

rpl Jun 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpl Jun 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jun 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harikishen commented Jun 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harikishen commented Jun 20, 2017

coveralls commented Jun 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jun 20, 2017

harikishen commented Jun 20, 2017

coveralls commented Jun 20, 2017

Choose a reason for hiding this comment

rpl Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jun 22, 2017

harikishen commented Jun 22, 2017

rpl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harikishen commented Jun 24, 2017

harikishen commented Jun 24, 2017

coveralls commented Jun 24, 2017

rpl commented Jun 30, 2017

rpl commented Apr 27, 2018

harikishen commented Jun 8, 2017 •

edited by kumar303

Loading

rpl Jun 15, 2017 •

edited

Loading

rpl Jun 15, 2017 •

edited

Loading

rpl left a comment •

edited

Loading

harikishen commented Jun 15, 2017 •

edited

Loading

rpl Jun 16, 2017 •

edited

Loading

rpl Jun 16, 2017 •

edited

Loading

rpl Jun 22, 2017 •

edited

Loading