olofk / edalize

An abstraction library for interfacing EDA tools
BSD 2-Clause "Simplified" License
642 stars 191 forks source link

test_vivado.py fails #132

Closed rodrigomelo9 closed 4 years ago

rodrigomelo9 commented 4 years ago
$ pytest test_vivado.py 
===================================================================================== test session starts ======================================================================================
platform linux -- Python 3.5.3, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
rootdir: /.../edalize
collected 2 items                                                                                                                                                                              

test_vivado.py F.                                                                                                                                                                        [100%]

=========================================================================================== FAILURES ===========================================================================================
_________________________________________________________________________________________ test_vivado __________________________________________________________________________________________

    def test_vivado():
        import os
        import shutil
        from edalize_common import compare_files, setup_backend, tests_dir

        ref_dir      = os.path.join(tests_dir, __name__)
        paramtypes   = ['generic', 'vlogdefine', 'vlogparam']
        name         = 'test_vivado_0'
        tool         = 'vivado'
        tool_options = {
            'part' : 'xc7a35tcsg324-1',
        }

        (backend, work_root) = setup_backend(paramtypes, name, tool, tool_options)
        backend.configure()

        compare_files(ref_dir, work_root, [
            'Makefile',
            name+'.tcl',
            name+'_run.tcl',
>           name+'_pgm.tcl',
        ])

test_vivado.py:23: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

ref_dir = '/.../edalize/tests/test_vivado', work_root = '/tmp/vivado_u01cvq93'
files = ['Makefile', 'test_vivado_0.tcl', 'test_vivado_0_run.tcl', 'test_vivado_0_pgm.tcl']

    def compare_files(ref_dir, work_root, files):
        import os.path
        import shutil

        for f in files:
            reference_file = os.path.join(ref_dir, f)
            generated_file = os.path.join(work_root, f)

            assert os.path.exists(generated_file)

            if 'GOLDEN_RUN' in os.environ:
                shutil.copy(generated_file, reference_file)

            with open(reference_file) as fref, open(generated_file) as fgen:
>               assert fref.read() == fgen.read(), f
E               AssertionError: test_vivado_0.tcl

edalize_common.py:22: AssertionError
-------------------------------------------------------------------------------------- Captured log call ---------------------------------------------------------------------------------------
WARNING  edalize.vivado:vivado.py:138 qip_file.qip has unknown file type 'QIP', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 qip_file.qip has unknown file type 'QIP', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 qsys_file has unknown file type 'QSYS', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 qsys_file has unknown file type 'QSYS', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 bmm_file has unknown file type 'BMM', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 bmm_file has unknown file type 'BMM', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 pcf_file.pcf has unknown file type 'PCF', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 pcf_file.pcf has unknown file type 'PCF', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 ucf_file.ucf has unknown file type 'UCF', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 ucf_file.ucf has unknown file type 'UCF', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 c_file.c has unknown file type 'cSource', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 c_file.c has unknown file type 'cSource', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 cpp_file.cpp has unknown file type 'cppSource', interpretation is up to Vivado
WARNING  edalize.vivado:vivado.py:138 cpp_file.cpp has unknown file type 'cppSource', interpretation is up to Vivado
=================================================================================== short test summary info ====================================================================================
FAILED test_vivado.py::test_vivado - AssertionError: test_vivado_0.tcl
================================================================================= 1 failed, 1 passed in 0.24s ==================================================================================

I made some diff to see changes:

Only in test_vivado: minimal
Only in /tmp/vivado_y4p0gmip/: test_vivado_0_synth.tcl
diff test_vivado/test_vivado_0.tcl /tmp/vivado_y4p0gmip/test_vivado_0.tcl
8,10c8,10
< set_property generic {vlogparam_bool=1 vlogparam_int=42 vlogparam_str=hello } [get_filesets sources_1]
< set_property generic {generic_bool=true generic_int=42 generic_str=hello } [get_filesets sources_1]
< set_property verilog_define {vlogdefine_bool=1 vlogdefine_int=42 vlogdefine_str=hello } [get_filesets sources_1]
---
> set_property generic {vlogparam_int=42 vlogparam_bool=1 vlogparam_str=hello } [get_filesets sources_1]
> set_property generic {generic_bool=true generic_str=hello generic_int=42 } [get_filesets sources_1]
> set_property verilog_define {vlogdefine_str=hello vlogdefine_bool=1 vlogdefine_int=42 } [get_filesets sources_1]
Only in test_vivado: vivado.cmd
$ diff test_vivado/minimal/ /tmp/vivado_rf5a0p4x/
Only in /tmp/vivado_rf5a0p4x/: test_vivado_minimal_0_synth.tcl
GCHQDeveloper560 commented 4 years ago

From #136 it looks like almost all of the tests, including this one, fail with Python 3.5 but pass for later versions. It works for me with 3.6, and looking at the CI run the only failure for 3.6 was for test_verible_lint_default that needs the reference data updated (#135).

It looks like we have a lot of cleanup to do if we want to support 3.5!

olofk commented 4 years ago

From the diff I see that the argument order has changed. I highly value reproducability to minimize unnecessary changes in the generated files between builds, so I would like this to be fixed. The dicts that contain the generic/vlogparam key/value pairs are already OrderedDicts, so they should be stable as long as the insertion order is unchanged.

This probably means that there have been some changes to the order if which the arguments are parsed. Perhaps the easiest way is just to ignore the insertion order and sort them just before they are actually written to the tcl file. This would mean a lot more places to patch, I guess. Also weird that only the vivado backend is affected by this. Not sure about anything right now

rodrigomelo9 commented 4 years ago

Hi @olofk.

Also weird that only the vivado backend is affected by this.

I only executed the vivado test, because it was where I was working (#133). I am using Debian 9, so my default Python is 3.5. @GCHQDeveloper560 commented::

From #136 it looks like almost all of the tests, including this one, fail with Python 3.5 but pass for later versions.

Regards

GCHQDeveloper560 commented 4 years ago

I've got a couple of fixes in my py35 branch that allow everything except test_vunit to pass for me.

There were a couple of places involving generics/parameters where OrderedDict wasn't used that broke test_vivado (and likely other) tests.

The vunit tests imported typing.Collections, which is new for 3.6, so this also caused errors. These were unused imports, so I just removed them. There's still a vunit failure I've not figured out, as there's a lot going on in test_vunit_hooks.

FAILED tests/test_vunit.py::test_vunit_hooks - AttributeError: assert_called_once

Any ideas on that one are appreciated, or I'll just submit a PR for these fixes.

olofk commented 4 years ago

That's fantastic! I would like to support Python 3.5 for still some time but worried it would be hard to fix. Many thanks. Please submit what you have. Worst case we just mark that particular vunit case as an expected fail when running with python 3.5

GCHQDeveloper560 commented 4 years ago

I looked at the vunit failure again. I did more unused import cleanup looking for the problem, but removing these didn't change anything. It looks like the actual problems were another OrderedDict needed to replace a dict comprehension and use of unittest.mock.assert_called_once() which is new for 3.6+.

The need to use OrderedDict for almost every dict in the tests where we're comparing files is going to make it difficult to keep the tests for 3.5 working unless we're careful.

I rebased the branch against master so that it applies cleanly and submitted #143

GCHQDeveloper560 commented 4 years ago

I see the tests include Python 2.7 and they're not surprisingly failing. I didn't look long but fixed the first layer of failures with the dict unpacking in test_quartus.py and lack of support for unittest.mock but there was another longer list of failures after that.

Does Edalize intend to support Python 2? If not should we add that restriction to setup.py and turn off the tests?

imphil commented 4 years ago

@GCHQDeveloper560 fusesoc remoed Python 2.7 support, there has been no discussion or decision on edalize yet. I've opened https://github.com/olofk/edalize/issues/144 to track that.

For now it's probably OK to disable the tests for Python 2.7 (as they are already broken), and then fix them or disable them depending on how we want to move on. Can you do that in #143?

imphil commented 4 years ago

All tests are green in CI now.