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.

Sign up for GitHub

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

Jump to bottom

Beyond unit testing #575

Open
imd1 opened this issue Apr 30, 2021 · 13 comments
Open

Beyond unit testing #575

imd1 opened this issue Apr 30, 2021 · 13 comments
Assignees
@jeremiah-c-leary
Labels
question

Comments

@imd1
Copy link
Contributor

imd1 commented Apr 30, 2021

What is your question?
I noticed that are hundreds of unit tests, but I wonder how much testing is performed at the "system" level, i.e. real world combinations of rules?

Is there any merit in running VSG on a real world set of files and at least checking it is able to auto-fix them without error?
@imd1 imd1 added the question label Apr 30, 2021
@jeremiah-c-leary
Copy link
Owner

Hey @imd1 ,

That has been the biggest hurdle with development, getting access to a wide range of VHDL code.

I do have a set of tests under vsg/tests/styles that run over some code I pulled down from open cores. They do not cover all the rules however.

Any ideas to improve this would be appreciated.

--Jeremy

@imd1
Copy link
Contributor Author

imd1 commented Oct 8, 2021 •
edited

@jeremiah-c-leary

Searched for trending github sites with vhdl tags:

A couple standout:

Could run the tool on these sites, and attempt to fix their code to flush out VSG crash bugs at least?

@imd1
Copy link
Contributor Author

imd1 commented Oct 8, 2021 •
edited

Could repeat search in gitlab?

@jeremiah-c-leary
Copy link
Owner

Could run the tool on these sites, and attempt to fix their code to flush out VSG crash bugs at least?

That is a good idea. I will look into it.

@jeremiah-c-leary jeremiah-c-leary added this to To do in Release 3.6.0 via automation Jan 2, 2022
@jeremiah-c-leary jeremiah-c-leary moved this from To do to In progress in Release 3.6.0 Jan 2, 2022
@jeremiah-c-leary
Copy link
Owner

I downloaded OSVVM and ran into an issue with procedure_410. If there was more than one space before a : on a single line parameter list, it would combine the procedure keyword and the procedure identifier.

Very strange.

jeremiah-c-leary added a commit that referenced this issue Jan 2, 2022
@jeremiah-c-leary
Update for issue #575:
4feb4bd 
Fixing alignment rules as they were introducing syntax errors:
  align_tokens_in_region_between_tokens
  align_tokens_in_region_between_tokens_skiping_lines_starting_with_tokens
@jeremiah-c-leary
Copy link
Owner

I have made it through OSVVM and UVVM.

@imd1
Copy link
Contributor Author

imd1 commented Jan 19, 2022

Good news...important to keep re running this as part of your testing, and ensure you go back to the original content to check VSG copes with fixing etc

@jeremiah-c-leary
Copy link
Owner

I made it through the code at this site:
https://github.com/VLSI-EDA/PoC

important to keep re running this as part of your testing, and ensure you go back to the original content to check VSG copes with fixing etc

I wonder how I can keep testing against these code bases. There are a lot of files and some of them are over 5000 lines. Maybe it is something I could run periodically....but then it would have to be local. I have not quite figured out the best way to incorporate it.

I have been focusing on parser errors at the moment and will have to go back and check on the style.

@jeremiah-c-leary
Copy link
Owner

I was able to get through the following code bases (I pulled the list from rust_hdl's github page):

https://github.com/VUnit/vunit
https://github.com/OSVVM/OSVVM
https://github.com/UVVM/UVVM
https://github.com/VLSI-EDA/PoC
https://github.com/FPHDL/fphdl/
https://github.com/inforichland/freezing-spice
https://github.com/sergeykhbr/riscv_vhdl
https://github.com/fabioperez/space-invaders-vhdl
https://github.com/BG2BKK/img_process_vhdl
https://github.com/kevinpt/vhdl-extras
https://github.com/xesscorp/VHDL_Lib

I think I will merge my pull request and do a release this weekend.

@jeremiah-c-leary
Copy link
Owner

Maybe I could have a file with a list of github links. Then I could have a script pull everything down and run VSG against it. Possibly before every release?

What do you think?

jeremiah-c-leary added a commit that referenced this issue Jan 23, 2022
@jeremiah-c-leary
Issue 575 (#703)
a18fe76 
* Update for issue #575:

Fixing alignment rules as they were introducing syntax errors:
  align_tokens_in_region_between_tokens
  align_tokens_in_region_between_tokens_skiping_lines_starting_with_tokens

* Refactoring to improve readability.

* Addressing issues from Codacy and Codeclimate

* Refactoring number_of_tokens_in_token_list to improve readability.

* Refactoring utils functions to improve readability.

* Removing unused variable.

* An exit statement with a <= was being classified as a signal assignment.

* Fixed issue with improper classification of character literals.

* Extracting class from tokens.py

* Refactoring tokens.py to improve readability.

* Fixing issue when attempting to detect trailing whitespace.

* Updating tests to reflect changes in tokens.py

* Refactoring combine_characters_into_words to improve readability.

* Refactoring combine_string_literals to improve readability.

* Refactoring find_indexes_of_double_quote_pairs to improve readability.

* Refactoring combine_character_literals to improve readability.

* Refactoring to improve readability.

* Removing unused functions.

* Adding additional tests for tokenizer.

* Moving rule variable_assignment_004 to phase 5 and subphase 2 to ensure it runs correctly.

* Updating documentation for variable_assignment_004 change in phase.

* Rule constant_012 was operating on constant which were not arrays.

* Removing unused function.

* Removing unused function.

* Moving rule generic_001 to entity_200 to properly handle generic_clause in a package declaration.

* Refactoring extract routines to improve readability.

* Refactoring base rule to improve readability.

* Fixing Minor formatting issues.

* Removing unused function.

* Removing unused functions.

* Improving code coverage.

* Improving code coverage:

  1)  Removed unused functions
  2)  Removed unreachable code

* Refactoring constant_012 to improve readability.

* Refactoring analyze to _analyze:

  1)  Updated _get_tokens_of_interest to add more info into the token object

* Refactoring to improve readability.

* Refactoring generate_paren_dicts function to improve readability.

* Refactoring check_indents to improve readability.

* Refactoring to improve readability.

* Updates for rule constant_012:

  1)  Arguments align_left False and align_paren True working

* Refactored analyze_align_left_true_align_paren_false function in rule constant_012

* Refactored _analyze_align_paren_true_align_left_true function for rule constant_012.

* Removed unused functions and methods for rule constant_012.

* Updated rule constant_012 to use the indent step

* Refactoring to functions into methods.

* Removing unused file.

* Refactoring rule constant_012 to improve readability.

* Additional refactoring to improve readability.

* Refactoring check_left_aligned_line function to use less parameters.

* Removing unused variables.

* Attempting to get JSON extract test to pass on CI server.

* Removing unused functions.

* Updates for rule comment_100:

  Rule was error out if a comment was only 3 characters and the third was not a letter or number.

    1) Updated test
    2) Added check for length of comment

* Updates for parsing procedure_calls_statements:

Procedure call with <= was being classified as a simple waveform.

  1)  Updated test to expose issue
  2)  Updated classification code

* Refactoring to reduce cyclomatic complexity.

* Refactoring to reduce cyclomatic complexity.

* Minor formatting updates.

* Adding parsing of physical_type_definitions:

  1)  Added tokens
      a)  physical_type_definition
      b)  primary_unit_declaration
      c)  secondary_unit_declaration
  2)  Added classifiers
      a)  physical_type_definition
      b)  primary_unit_declaration
      c)  secondary_unit_declaration
  3)  Added test
      a)  full_type_declaration

* Minor formatting fix.

* Fixed bug with concurrent_simple_signal_assignment.

  A <= in a generic map was causing the generic map to be classified as a concurrent_simple_signal_assignment.

  1)  Updated test
  2)  Updated concurrent_simple_signal_assignment classifier

* Refactoring to reduce cyclomatic complexity.

* Adding classification of configuration_specification:

  1)  Added tests
  2)  Added classifiers for configuration_specification.

* Minor formatting updates.

* Fixing issue with previous_line rule if there were no lines above.

* Assert_statement was being classified before exiting block_statement.

* A backslash in a string literal was not be classified as a string literal.
@imd1
Copy link
Contributor Author

imd1 commented Jan 23, 2022

Maybe I could have a file with a list of github links. Then I could have a script pull everything down and run VSG against it. Possibly before every release?

What do you think?

Many options...you could run something on your CI server, maybe once a week on the master branch, or maybe run it on every merge/commit to master?

@jeremiah-c-leary
Copy link
Owner

Many options...you could run something on your CI server, maybe once a week on the master branch, or maybe run it on every merge/commit to master?

What if I did this:

  1. Clone code repo twice
    a) First would be called the reference code
    b) Second I would apply a style and commit the changes. This would be called the fixed code.
  2. Create a test
    a) Include the repo URL for the reference and fixed code
    b) Pull down reference code and fixed code URLs
    c) Run VSG with style to reference code
    d) Compare fixed reference to fixed code

I worry this could take a lot of effort, but maybe it is what needs to be done.

@jeremiah-c-leary jeremiah-c-leary removed this from In progress in Release 3.6.0 Feb 1, 2022
@jeremiah-c-leary jeremiah-c-leary self-assigned this Feb 1, 2022
@imd1
Copy link
Contributor Author

imd1 commented Feb 6, 2022

The main problems I've found with "real" code is VSG crashing or VSG completing but leaving malformed VHDL behind. I think your testing should concentrate on these two problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeremiah-c-leary jeremiah-c-leary

Labels
question
Projects
vhdl-style-guide
Status: Triaged
Milestone
No milestone
Development

No branches or pull requests
2 participants
@imd1 @jeremiah-c-leary