chaoss / grimoirelab-graal

A Generic Repository AnALyzer
GNU General Public License v3.0
21 stars 62 forks source link

[cloc] Fix cloc error due to mulitple word language-name #46

Closed inishchith closed 5 years ago

inishchith commented 5 years ago

@valeriocos As I was working on large repository evaluation(https://github.com/inishchith/gsoc-graal/issues/12#issuecomment-513395968), enter this error in logic under cloc analyzer which depends on reporting of results based on position in the list.

This PR fixes the error by picking the elements from the last(right to left). Please let me know what you think :)

Signed-off-by: inishchith inishchith@gmail.com

inishchith commented 5 years ago

@valeriocos

Could you increase the version number of the cloc analyzer?

Sure. Done!

It would be possible to add some assertions to the existing tests (test_cloc.py) to be sure we are correctly parsing blanks, comments and loc?

It's possible by implementing the same logic at the test side. I'll add it, Please let me know if there's a better way of doing it.

Thanks!

valeriocos commented 5 years ago

Sure. Done!

Thanks

It's possible by implementing the same logic at the test side. I'll add it, Please let me know if there's a better way of doing it.

I apologize for being unclear. I meant to do something like:

    def test_analyze(self):
        """Test whether cloc returns the expected fields data"""

        cloc = Cloc()
        kwargs = {'file_path': os.path.join(self.data_path, ANALYZER_TEST_FILE)}
        result = cloc.analyze(**kwargs)

        self.assertIn('blanks', result)
        self.assertTrue(type(result['blanks']), int)
        self.assertTrue(result['blanks'], 27) <--
        self.assertIn('comments', result)
        self.assertTrue(type(result['comments']), int)
        self.assertTrue(result['comments'], 31) <--
        self.assertIn('loc', result)
        self.assertTrue(type(result['loc']), int)
        self.assertTrue(result['loc'], 67) <--
inishchith commented 5 years ago

@valeriocos No worries. Thanks for the clarification! I've made the changes.