range_test and sge_test not picked up by tox due to being executable

[lnsndn]

While developing some new functionality in luigi.tools.range I ran the range_test.py file manually and noticed that it had multiple failing tests that were unrelated to my changes, even on a clean master with no changes at all. Strangely, tox goes through and the build passes anyway. This happens because the whole file is ignored. The reason the file is ignored is because it is set as executable (755) which apparently causes tox to ignore it.

Steps to reproduce

1. Run complete test suite as usual and watch everything go through: tox
2. Run only the range_test.py file and it has multiple issues, FAILED (failures=2, errors=6): cd test && python -m unittest range_test
3. Change file permissions: chmod 644 range_test.py
4. Run tox suite again and this time the file is picked up, failing with the same problems:

======================================================================
ERROR: Verify that you can still programmatically set of param as string
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1385, in test_old_month_instantiation
    self.assertEqual(expected_task, list(range_task._requires())[0])
IndexError: list index out of range

======================================================================
ERROR: test_param_name (range_test.MonthInstantiationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1423, in test_param_name
    self.assertEqual(expected_task, list(range_task._requires())[0])
IndexError: list index out of range

======================================================================
ERROR: test_param_name_with_inferred_fs (range_test.MonthInstantiationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1439, in test_param_name_with_inferred_fs
    self.assertEqual(expected_task, list(range_task._requires())[0])
IndexError: list index out of range

======================================================================
FAIL: Verify that you can still use Range through CLI
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1407, in test_month_cli_instantiation
    self.assertEqual(MyTask(month_param=datetime.date(1934, 12, 1)).secret, 'yay')
AssertionError: 'some-value-to-sooth-python-linters' != 'yay'
-------------------- >> begin captured logging << --------------------
luigi: INFO: logging already configured
luigi.scheduler: DEBUG: Starting pruning of task graph
luigi.scheduler: DEBUG: Done pruning task graph
--------------------- >> end captured logging << ---------------------

======================================================================
FAIL: test_of_param_commandline (range_test.MonthInstantiationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1481, in test_of_param_commandline
    self.assertEqual(MyTask.state, ('bar', 5))
AssertionError: Tuples differ: (None, None) != ('bar', 5)

First differing element 0:
None
'bar'

- (None, None)
+ ('bar', 5)

======================================================================
ERROR: test_non_dividing_interval (range_test.RangeByMinutesBaseTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 626, in test_non_dividing_interval
    self.assertRaises(luigi.parameter.ParameterException, task.requires)
  File "/usr/lib/python3.6/unittest/case.py", line 733, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/usr/lib/python3.6/unittest/case.py", line 178, in handle
    callable_obj(*args, **kwargs)
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 240, in requires
    self._emit_metrics(missing_datetimes, finite_start, finite_stop)
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 177, in _emit_metrics
    delay_in_jobs = len(datetimes) - datetimes.index(missing_datetimes[0]) if datetimes and missing_datetimes else 0
ValueError: datetime.datetime(2016, 3, 31, 0, 0) is not in list

======================================================================
ERROR: test_bulk_complete_correctly_interfaced (range_test.RangeByMinutesTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1463, in test_bulk_complete_correctly_interfaced
    actual = [str(t) for t in task.requires()]
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 233, in requires
    missing_datetimes = sorted(self._missing_datetimes(datetimes))
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 272, in _missing_datetimes
    return self.missing_datetimes(finite_datetimes)
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 798, in missing_datetimes
    complete_parameters = self.of.bulk_complete.__func__(cls_with_params, map(self.datetime_to_parameter, finite_datetimes))
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1447, in bulk_complete
    return parameter_tuples[:-2]
TypeError: 'map' object is not subscriptable

======================================================================
ERROR: test_bulk_complete_of_params (range_test.RangeByMinutesTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1493, in test_bulk_complete_of_params
    actual = [str(t) for t in task.requires()]
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 233, in requires
    missing_datetimes = sorted(self._missing_datetimes(datetimes))
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 272, in _missing_datetimes
    return self.missing_datetimes(finite_datetimes)
  File "/home/lena/virtualenvs/luigi/lib/python3.6/site-packages/luigi-2.8.7-py3.6.egg/luigi/tools/range.py", line 798, in missing_datetimes
    complete_parameters = self.of.bulk_complete.__func__(cls_with_params, map(self.datetime_to_parameter, finite_datetimes))
  File "/home/lena/projects/github/spotify/luigi/test/range_test.py", line 1476, in bulk_complete
    return parameter_tuples[:-2]
TypeError: 'map' object is not subscriptable

The only executable files I could find were range_test.py and contrib/sge_test.py.

Solution

We need to chmod the concerned files to match the others (644) and fix the failing tests. If we can't fix them right now we need to temporarily ignore the individual tests and create new Github issues. I am in progress of doing this but wanted to report the error in case anyone else encounters the problem.

[lnsndn]

@lallea FYI

[lallea]

Thanks @lnsndn for debugging and fixing. Probably my bad.

I reviewed and concluded that the tests are wrong, but the implementation is right. The tests were probably passing before #2666 got merged.

[Tarrasch]

Nice catch!!

[lnsndn]

Thank you @Tarrasch!

By the way, isn't this error kind of problematic, that tox just silently ignores executable files? How would we prevent it from happening again? I can definitely see someone accidentally modifying file permissions again, tests go through and no one notices anything.

[Tarrasch]

It is worrying yes. Not sure If I know a general solution to this though. (that's also practical :))