Skip to content
This repository has been archived by the owner on Dec 10, 2019. It is now read-only.

(feature): add code coverage #6

Merged
merged 1 commit into from
Mar 6, 2018
Merged

(feature): add code coverage #6

merged 1 commit into from
Mar 6, 2018

Conversation

cgalvarez
Copy link
Contributor

@cgalvarez cgalvarez commented Mar 5, 2018

Here you have it, @jccguimaraes . This PR adds code coverage to the package through atom-coverage.

To test locally just:

git clone -b new/add-code-coverage [email protected]:cgalvarez/project-viewer-plus.git
npm install
npm run test:coverage

It should output something like:

File %Stmts %Branch %Funcs %Lines Uncovered Lines
All files 30.73 25.21 26.91 30.54
--lib 41.3 10 25 41.3
----main.js 100 100 100 100
----package.js 41.3 10 25 41.3 ...143,164,173
--lib/components 15.12 3.33 15.38 14.12
----empty.js 53.85 25 50 53.85 ...29,32,46,53
----group.js 0 0 0 0 ...83,91,94,96
----icon.js 0 0 0 0 ...72,80,92,96
----main.js 54.55 0 60 50 26,27,28,31,38
----project.js 0 0 0 0 ...77,90,93,95
----select-list-item.js 0 0 0 0 ...36,44,45,49
--lib/constants 100 100 100 100
----base.js 100 100 100 100
----config.js 100 100 100 100
----icons.js 100 100 100 100
--lib/containers 32.79 30.23 32.76 33.06
----editor.js 0 0 0 0 ...146,154,162
----icons.js 0 0 0 0 ...67,68,74,82
----list.js 38.89 25 40 38.89 ...60,63,64,66
----main.js 54.55 50 52.63 56.25 ...177,185,192
----select-list.js 44.12 50 46.67 44.12 ...136,137,145
--lib/services 40.09 38.14 38.67 39.83
----context-switcher.js 7.81 8.33 5 7.81 ...183,184,185
----database.js 39.25 21.74 35.14 39.25 ...381,382,383
----devlog.js 66.67 50 100 66.67 3
----github.js 100 100 100 100
----packages.js 100 100 100 100
----state.js 73.58 69.57 81.25 73.08 ...148,157,171
--lib/services/packages 0 0 0 0
----find-and-replace.js 0 0 0 0 ...48,50,51,54
----linter-ui-default.js 0 0 0 0 ...45,47,48,51
----linter.js 0 0 0 0 ...45,47,48,51
----status-bar.js 0 0 0 0 ...34,37,38,40
----tree-view.js 0 0 0 0 ...140,143,147

BTW, the reason why the CI builds fail is not related with this PR (at least right now), but to tests failing.

@jccguimaraes
Copy link
Owner

hey @cgalvarez !

Thank you so much! Great work mate. Yes, actually I know why it fails :) Just haven't managed the time to try to fix this.

What is your opinion on this project? Does it converge with your ideas?

@jccguimaraes jccguimaraes merged commit 4e9036a into jccguimaraes:master Mar 6, 2018
@cgalvarez
Copy link
Contributor Author

@jccguimaraes Well, still haven't dig into your project. Right now I'm upgrading my fork to the latest available package versions, refactoring code, writing specs, learning MobX...

Until finished polishing it, won't take a deeper look into yours. I'll provide my feedback to you as soon as I understand what it does and give it a try, but according to atom-project-viewer's README (I think this project is a massive rewrite of that package), it focuses on organizing and managing projects, so its scope should be delimited to that.

What I try to achieve right now with atom-projects is customizing and improving the developer experience with Atom for each specific project. For example, let's assume one of your projects is a WordPress plugin. You'll want to have some packages as ide-php, php-debug and linter-php enabled while developing it. My first goal with atom-projects would be enable only those packages when you have that project open, so Atom runs smoother with less packages enabled (even defining presets that could be shared between developers, like ESLint configs).

Since atom-projects is more general (its name), I think both could fit well on it and complement each other. But being honest, I need to polish mine first, since I have no experience with MobX, Etch or React. I hope then I will be able to understand what yours does much better 😉

@cgalvarez
Copy link
Contributor Author

BTW, while creating this PR I had to improve atom-coverage to work with shorthands or long names for Babel presets/plugins, and providing them with options, thanks to your config in .babelrc, so thank you very much @jccguimaraes for helping me to learn something new!

@jccguimaraes
Copy link
Owner

@cgalvarez that is what OS is all about 👍 learning from each other!

@cgalvarez
Copy link
Contributor Author

@jccguimaraes Mocha already supports waiting for promises out-of-the-box by returning them or using the callback done(), but while migrating tests for my package yesterday, I realized that I need a drop-in replacement for Jasmine's waitsFor()/runs() and Atom's waitsForPromise().

I have some methods that perform async tasks without directly returning a promise, so I've created the helper waitsForCondition(isSatisfied: Function, msMaxWait: number = 500, msInterval: number = 25), which returns a promise that calls isSatisfied() every msInterval milliseconds to check if the desired condition is met/satisfied. If so, the promise is resolved. If not satisfied under msMaxWait milliseconds, then the promise is rejected.

I think this allows a clearer syntax when writing specs. For example:

it('should do async task', () => {
  myVar.asyncTask();
  return waitsForCondition(() => (myVar.asyncFlag === true))
    .then(() => {
      expect(myVar).to...
    });
});

If you think you can benefit of this, or has any other suggestions (I'm not an expert on writing Atom specs), please, let me know and I'll add this helper to atom-coverage so you can use it 😉 , and will make a new PR to upgrade to the new released version.

@cgalvarez cgalvarez mentioned this pull request Mar 7, 2018
10 tasks
jccguimaraes added a commit that referenced this pull request May 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants