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

Mark array_axis_vcs and verilog_ams examples as xfail #751

Merged
merged 7 commits into from
Oct 8, 2021
Merged

Conversation

umarcor
Copy link
Member

@umarcor umarcor commented Oct 2, 2021

This PR uses pytest's xfail mark (https://docs.pytest.org/en/6.2.x/skipping.html), for allowing example array_axis_vcs and verilog_ams to fail when running the acceptance tests, without the overall result being a failure.
By the way, instead of using skipIf and skipUnless from unittest, the skipif mark from pytest is used as well.

Context: #747 (comment)

@LarsAsplund
Copy link
Collaborator

I think this would be improved if we enforce the reporting the reason for skipping and xfailing. I experimented a bit and if you add an conftest.py file with

def pytest_configure(config):
    config.option.reportchars = "xXs"

the reason will always be visible:

$ pytest test_external_run_scripts.py -k "axis or ams"
============================================================================================================ test session starts ============================================================================================================= platform win32 -- Python 3.9.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: C:\github\vunit
collected 42 items / 40 deselected / 2 selected

test_external_run_scripts.py sx                                                                                                                                                                                                         [100%]

========================================================================================================== short test summary info ===========================================================================================================
XFAIL test_external_run_scripts.py::TestExternalRunScripts::test_vhdl_array_axis_vcs_example_project
  Only simulators with PSL functionality
SKIPPED [1] test_external_run_scripts.py:46: Verilog
=============================================================================================== 1 skipped, 40 deselected, 1 xfailed in 32.75s ================================================================================================

The reason "Verilog" should probably be a bit more elaborate...

These changes will cause unittest to fail. I'm ok with only supporting pytest. If so, we need to update the docs that only recommends pytest.

@umarcor
Copy link
Member Author

umarcor commented Oct 3, 2021

I think this would be improved if we enforce the reporting the reason for skipping and xfailing. I experimented a bit and if you add an conftest.py file with

We are using -ra in https://github.com/VUnit/vunit/blob/master/pyproject.toml#L44-L47. Hence, if tests are executed through tox, it should show the reasons. See https://docs.pytest.org/en/latest/how-to/output.html#producing-a-detailed-summary-report:

Here is the full list of available characters that can be used:

  • f - failed
  • E - error
  • s - skipped
  • x - xfailed
  • X - xpassed
  • p - passed
  • P - passed with output

Special characters for (de)selection of groups:

  • a - all except pP
  • A - all
  • N - none, this can be used to display nothing (since fE is the default)

For example: https://github.com/VUnit/vunit/runs/3775284015?check_suite_focus=true#step:3:124

The reason "Verilog" should probably be a bit more elaborate...

I changed it to "Requires a Verilog simulator".

These changes will cause unittest to fail. I'm ok with only supporting pytest. If so, we need to update the docs that only recommends pytest.

I updated the docs accordingly.

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

Successfully merging this pull request may close these issues.

None yet

3 participants