Paths

Table of Contentst

Differential D1646

swh.lister.cran: Add description in task_dict
ClosedPublic
Actions

Authored by nahimilega on Jun 26 2019, 4:08 PM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Commits

rDLS5ea9d5ed392a: swh.lister.cran: Add description in task_dict

Summary

Add description in task_dict method because
the only metadata that can be found for a
package at CRAN is its decsription. That can
only br achived from the build in API in R,
which ister is already using. Hence instead of
getting metadata in loader, it is passed
by lister

Diff Detail

Repository

rDLS Listers

Branch

cran

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 6516
Build 9076: tox-on-jenkins	Jenkins
Build 9075: arc lint + arc unit

Event Timeline

nahimilega created this revision.Jun 26 2019, 4:08 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 26 2019, 4:08 PM

nahimilega added a subscriber: vlorentz.Jun 26 2019, 4:12 PM

nahimilega added a subscriber: ardumont.

Build is green
See https://jenkins.softwareheritage.org/job/DLS/job/tox/277/ for more details.

Harbormaster completed remote builds in B6509: Diff 5516.Jun 26 2019, 4:13 PM

nahimilega added inline comments.Jun 26 2019, 4:16 PM

swh/lister/cran/tests/test_tasks.py
38–41	I an not sure this this the correct way of adding the tests which were mentioned by @vlorentz in D1644. Please guide me the correct way.

No, when you do this, mock_create_tasks.assert_called_once_with() checks it was called a single time, and that single time is mock_create_tasks(). So what you are actually testing is that swh.scheduler.utils.create_task_dict is never called by the lister itself.

In D1646#37925, @vlorentz wrote:

No, when you do this, mock_create_tasks.assert_called_once_with() checks it was called a single time, and that single time is mock_create_tasks(). So what you are actually testing is that swh.scheduler.utils.create_task_dict is never called by the lister itself.

Oh ok, so how should I write the test case to check that description is not deleted in future.
I tried this but it is also not working

with patch('swh.scheduler.utils.create_task_dict') as mock_create_tasks:
    lister.task_dict()
mock_create_tasks.assert_called_once_with()

Can you please help me with this

What is the error message?

In D1646#37927, @vlorentz wrote:

What is the error message?

(swh) archit@work-pc:~/swh-environment/swh-lister/swh/lister/cran$ tox .

...
swh/lister/cran/tests/test_tasks.py .F                                 [100%]

================================== FAILURES ==================================
________________________________ test_lister _________________________________

lister = <MagicMock name='CRANLister' id='140463269756880'>
swh_app = <Celery celery.tests at 0x7fc028e83cf8>
celery_session_worker = <Worker: gen7127@work-pc (running)>

    @patch('swh.lister.cran.tasks.CRANLister')
    def test_lister(lister, swh_app, celery_session_worker):
        # setup the mocked CRANLister
        lister.return_value = lister
        lister.run.return_value = None
    
        res = swh_app.send_task(
            'swh.lister.cran.tasks.CRANListerTask')
        assert res
        res.wait()
        assert res.successful()
    
        lister.assert_called_once_with()
        lister.db_last_index.assert_not_called()
        lister.run.assert_called_once_with()
    
        with patch('swh.scheduler.utils.create_task_dict') as mock_create_tasks:
            lister.task_dict()
>       mock_create_tasks.assert_called_once_with()

swh/lister/cran/tests/test_tasks.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

_mock_self = <MagicMock name='create_task_dict' id='140463269630696'>
args = (), kwargs = {}
self = <MagicMock name='create_task_dict' id='140463269630696'>
msg = "Expected 'create_task_dict' to be called once. Called 0 times."

    def assert_called_once_with(_mock_self, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        self = _mock_self
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times." %
                   (self._mock_name or 'mock', self.call_count))
>           raise AssertionError(msg)
E           AssertionError: Expected 'create_task_dict' to be called once. Called 0 times.

/usr/lib/python3.7/unittest/mock.py:839: AssertionError

It means swh.scheduler.utils.create_task_dict was never called.

This is because the lister imports from swh.scheduler.utils import create_task_dict before it was patched. Instead, you should patch swh.lister.cran.lister.create_task_dict.

In D1646#37937, @vlorentz wrote:

It means swh.scheduler.utils.create_task_dict was never called.

This is because the lister imports from swh.scheduler.utils import create_task_dict before it was patched. Instead, you should patch swh.lister.cran.lister.create_task_dict.

This I tried with patch swh.lister.cran.lister.create_task_dict.

with patch('swh.lister.cran.lister.create_task_dict') as mock_create_tasks:
    lister.task_dict()
mock_create_tasks.assert_called_once_with()

But still the same error showing mock_create_tasks never called

    @patch('swh.lister.cran.tasks.CRANLister')
    def test_lister(lister, swh_app, celery_session_worker):
        # setup the mocked CRANLister
        lister.return_value = lister
        lister.run.return_value = None
    
        res = swh_app.send_task(
            'swh.lister.cran.tasks.CRANListerTask')
        assert res
        res.wait()
        assert res.successful()
    
        lister.assert_called_once_with()
        lister.db_last_index.assert_not_called()
        lister.run.assert_called_once_with()
    
        with patch('swh.lister.cran.lister.create_task_dict') as mock_create_tasks:
            lister.task_dict()
>       mock_create_tasks.assert_called_once_with()

swh/lister/cran/tests/test_tasks.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

_mock_self = <MagicMock name='create_task_dict' id='140059418513592'>
args = (), kwargs = {}
self = <MagicMock name='create_task_dict' id='140059418513592'>
msg = "Expected 'create_task_dict' to be called once. Called 0 times."

    def assert_called_once_with(_mock_self, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        self = _mock_self
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times." %
                   (self._mock_name or 'mock', self.call_count))
>           raise AssertionError(msg)
E           AssertionError: Expected 'create_task_dict' to be called once. Called 0 times.

Got it, it's because you added that check in a test where swh.lister.cran.tasks.CRANLister is mocked, so when you do lister.run() nothing actually happens. (You can check, even lister.thisIsNotARealFunction() wouldn't crash)

In D1646#37939, @vlorentz wrote:

Got it, it's because you added that check in a test where swh.lister.cran.tasks.CRANLister is mocked, so when you do lister.run() nothing actually happens. (You can check, even lister.thisIsNotARealFunction() wouldn't crash)

True I tested it.

To avoid this I made another function like this

def test_task_dict(lister):
    with patch('swh.lister.cran.lister.create_task_dict') as mock_create_tasks:
        lister.task_dict()
    mock_create_tasks.assert_called_once_with()

But is giving error showing fixture 'lister' not found

collected 3 items                                                              

swh/lister/cran/tests/test_tasks.py .E.                                  [100%]

==================================== ERRORS ====================================
______________________ ERROR at setup of test_description ______________________
file /home/archit/swh-environment/swh-lister/swh/lister/cran/tests/test_tasks.py, line 13
  def test_description(lister):
E       fixture 'lister' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, celery_app, celery_config, celery_enable_logging, celery_includes, celery_parameters, celery_session_app, celery_session_worker, celery_worker, celery_worker_parameters, celery_worker_pool, cov, depends_on_current_app, doctest_namespace, monkeypatch, no_cover, postgresql, postgresql_proc, pytestconfig, record_property, record_xml_attribute, record_xml_property, recwarn, requests_mock, swh_app, swh_scheduler, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, use_celery_app_trap
>       use 'pytest --fixtures [testpath]' for help on them.

/home/archit/swh-environment/swh-lister/swh/lister/cran/tests/test_tasks.py:13

Sorry, I am sure it is pretty easy to fix but I am still not able to get it.

Remove the argument of the test

In D1646#37941, @vlorentz wrote:

Remove the argument of the test

If lister argument is removed then it gives error 'lister' is not defined

    def test_task_dict():
        with patch('swh.lister.cran.lister.create_task_dict') as mock_create_tasks:
>           lister.task_dict()
E           NameError: name 'lister' is not defined

You must instantiate CRANLister

Add test to avoid removal of description in future

Build is green
See https://jenkins.softwareheritage.org/job/DLS/job/tox/280/ for more details.

Harbormaster completed remote builds in B6516: Diff 5523.Jun 27 2019, 11:13 AM

Cool!

Now, could you move it to a different file (it does not test the lister task itself, but a core feature of the lister)?
Also, could you add another test where the project_metadata is not empty?