NASA-PDS / roundup-action

Do a "roundup", a/k/a PDS-style continuous integration and delivery
Apache License 2.0
1 stars 4 forks source link

Tox build "failure" doesn't halt the build #70

Closed MJJoyce closed 2 years ago

MJJoyce commented 2 years ago

Noticed this in sandbox land on a build that failed (due to a separate issue).

Screen Shot 2021-09-30 at 16 04 08

The tox lint check fails but the build carries on without a hiccup.

nutjob4life commented 2 years ago

@MJJoyce got a link to the build log? Is this in epitome?

nutjob4life commented 2 years ago

@MJJoyce the logic right now is:

class _UnitTestStep(_PythonStep):
    '''A step to take with Python unit'''
    def execute(self):
        _logger.debug('Python unit test step')
        try:
            _logger.debug('Trying the new way: ``tox``')
            invoke(['tox'])
        except (InvokedProcessError, FileNotFoundError):
            _logger.debug("OK, the new way didn't work, trying the old way")
            invoke(['python', 'setup.py', 'test'])

So if tox failed I guess then python setup.py test succeeded.

For a true unit test step, perhaps the correct invocation is tox test?

MJJoyce commented 2 years ago

Hey @nutjob4life,

Here's a sample run where this happens https://github.com/nasa-pds-engineering-node/epitome/runs/3773100211?check_suite_focus=true

tox -e py39 would give us a pytest only run given the way I defined the environments. Not the most obvious I suppose ...

[tox]
envlist = py39, docs, lint
isolated_build = True

[testenv]
deps = .[dev]
whitelist_externals = pytest
commands = pytest

It does look like the tox build fails and it follows up with setup.py test. So, I suppose we need to sanity check that tox exists and fall back to setup.py test when it doesn't. Otherwise, we should believe tox when it fails.

MJJoyce commented 2 years ago

Failing like we expect! https://github.com/nasa-pds-engineering-node/epitome/actions/runs/1317276222