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

Jump to bottom

dist/tools/compile_and_test_for_board: add basic checks for script #10837

Merged
merged 3 commits into from Jan 23, 2019
Merged

dist/tools/compile_and_test_for_board: add basic checks for script #10837

merged 3 commits into from Jan 23, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jan 22, 2019

Contribution description

This PR is adding basic Python checks for the compile_and_test_for_board.py script. The checks are run using Tox. Performed are:

  • doctest (via pytest)
  • pylint
  • flake8

To avoid issues with pytest automatic parsing, I renamed the TestError exception to ErrorInTest.

Testing procedure

  • Install tox:

    pip install tox --user

  • Use it:

    cd dist/tools/compile_and_test_for_board
    tox compile_and_test_for_board.py

Issues/PRs references

Follow-up of #10818 and idea suggested by @cladmi

@aabadie aabadie added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools labels Jan 22, 2019
@aabadie aabadie requested a review from cladmi January 22, 2019 08:05
@aabadie aabadie mentioned this pull request Jan 22, 2019
Merged
@aabadie aabadie changed the title dist/tools/compile_and_test_for_board: add basic script checks dist/tools/compile_and_test_for_board: add basic checks for script Jan 22, 2019
cladmi
cladmi reviewed Jan 23, 2019
View reviewed changes
dist/tools/compile_and_test_for_board/README.md Outdated
Use [tox](https://tox.readthedocs.io/en/latest/) to run basic checks on the
script:

$ tox compile_and_test_for_board.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it have the script as argument ?
Every script could need its own tox file as the script may have its dependencies anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it have the script as argument ?

I find this more generic this way. We could reuse this tox config file for any python script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But a script that has dependencies or special targets needs his own file anyway. So I prefer it the new way :)

cladmi
cladmi suggested changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested to add this file so am obviously for it.
I would however prefer not running it with tox script_name but just tox as it is the normal usage for tox.

I like having 3 commits.

@aabadie
tools/compile_test_one_board: rename TestError exception class
26a6469 
Otherwise it conflicts with pytest automatic parsing which tries to execute all classes with name beginning with Test
@aabadie aabadie force-pushed the pr/tools/compile_test_board_checks branch from a43e857 to b7b8055 Compare January 23, 2019 15:25
@aabadie
Copy link
Contributor Author

aabadie commented Jan 23, 2019

I would however prefer not running it with tox script_name but just tox as it is the normal usage for tox.

I changed that, let me know what you think

cladmi
cladmi approved these changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, is good for me right now. Also I tested all 4 cases.

@cladmi
Copy link
Contributor

cladmi commented Jan 23, 2019

Can you squash and enable murdock when done ?

@aabadie
tools/compile_test_one_board: add tox.ini configuration file
148f715 
tox.ini configures 3 checks on the python script: doctest (via pytest), pylint and flake8
@aabadie
tools/compile_test_one_board: update documentation README
613e05f 
Provide some notes on how to perform basic checks on the script using tox
@aabadie aabadie force-pushed the pr/tools/compile_test_board_checks branch from b7b8055 to 613e05f Compare January 23, 2019 17:23
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 23, 2019
@aabadie
Copy link
Contributor Author

aabadie commented Jan 23, 2019

Can you squash and enable murdock when done ?

just did that

@aabadie
Copy link
Contributor Author

aabadie commented Jan 23, 2019

CI is green, let's merge!

@aabadie aabadie merged commit 3974c46 into RIOT-OS:master Jan 23, 2019
@aabadie aabadie added this to the Release 2019.01 milestone Jan 24, 2019
@aabadie aabadie deleted the pr/tools/compile_test_board_checks branch January 24, 2019 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cladmi cladmi cladmi approved these changes

Assignees
No one assigned
Labels
Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2019.01
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@aabadie @cladmi