Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coveralls integration + Codeception #44

Open
david-sosa-valdes opened this issue May 26, 2016 · 11 comments
Open

Coveralls integration + Codeception #44

david-sosa-valdes opened this issue May 26, 2016 · 11 comments
Assignees

Comments

@david-sosa-valdes
Copy link
Contributor

So i recently acquire some experience using coveralls.io.

I think we could improve the tests using Codeception and make a code-coverage report to Coveralls, so the users can explore all the acceptance and unit tests that we realize and see the latest code-coverage statistics for this project.

Do you like the idea? (if you like the idea i can start working on a pull-request)

https://coveralls.io/features

@vendethiel
Copy link
Owner

For sure. The tests are currently very ad-hoc, and their only "good" point is that "it's better than no tests at all"...

@david-sosa-valdes
Copy link
Contributor Author

Excellent!!

@david-sosa-valdes david-sosa-valdes self-assigned this May 26, 2016
@david-sosa-valdes
Copy link
Contributor Author

I have this fork using Codeception with the same tests you define and add some instructions to execute them in the README file,

The current report summary:

  • Classes: 28.00% (7/25)
  • Methods: 38.20% (34/89)
  • Lines: 50.70% (361/712) <-- coveralls use this porcentage

@david-sosa-valdes
Copy link
Contributor Author

Now we need to improve this coverage report by adding more tests by each component.

@david-sosa-valdes
Copy link
Contributor Author

I found some interesting points in Locator class:

Methods Tests
save none
skipFile File::skipDirective()
getPathsHash none
resolveFile none
hasDirectory none
getDirectoriesFor Filter\Less

This report concludes that 4 methods (save, getPathsHash, resolveFile and hasDirectory) are not used by any class and the 2 other methods need some tests using another classes.

The 4 methods mentioned doesn't have documentation, if we remove them we can improve the coveralls percentage by +2.53% (53.23%), do you think it's possible to delete these methods?

@vendethiel
Copy link
Owner

Mh, some of those are very useful at least. Need more tests.

@david-sosa-valdes
Copy link
Contributor Author

I'm still unfamiliarzed with some aspects of this lib, i'll test all this methods.

@vendethiel
Copy link
Owner

vendethiel commented Jun 4, 2016

  • save: This method is very important to cache the location of files. This makes it so that *= require x over and over won't need to look up where the file actually is all the time.
  • skipFile: Allows to exclude some files, so that they won't actually be required e.g. when requiring a whole directory.
  • getPathsHash: I don't remember, tbh. I think it's supposed to be used by getFileListName.
  • resolveFile: I don't remember what this was used for.
  • getDirectoriesFor: That's here to stay, but we should definitely add some tests that make use of it. i.e. import from LESS, to prove that the import paths were used for.

@david-sosa-valdes
Copy link
Contributor Author

I have managed to achieve 60.88% in coveralls implementing cache tests.

davidsosavaldes/Sprockets-PHP@052b180

@vendethiel
Copy link
Owner

Nice. The only thing I'm afraid is that including bootstrap in the tests will make finding the issue much harder, whenever something changes.

@david-sosa-valdes
Copy link
Contributor Author

I was trying to use some filters but we can try something else, the bootstrap tests are deleted on commit: davidsosavaldes/Sprockets-PHP@b040cd5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants