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

tests: remove leftover test targets in test application makefiles #10887

Merged
merged 3 commits into from
Jan 28, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jan 28, 2019

Contribution description

This is removing leftover test targets in test applications. Found while cleaning up generated applications after running the full test-suite.

Testing procedure

  • run make distclean > /dev/null from the riot directory
  • with master, you get something like:
Makefile:11: warning: overriding recipe for target 'test'
...tests/cond_order/../../Makefile.include:568: warning: ignoring old recipe for target 'test'
/bin/sh: 1: Syntax error: Missing '))'
Makefile:41: warning: overriding recipe for target 'test'
...tests/pkg_cmsis-dsp/../../Makefile.include:568: warning: ignoring old recipe for target 'test'
Makefile:9: warning: overriding recipe for target 'test'
...tests/pkg_lora-serialization/../../Makefile.include:568: warning: ignoring old recipe for target 'test'

  • with this PR, you now get something like:
/bin/sh: 1: Syntax error: Missing '))'

The syntax error comes from a bug with how the ROM_LEN is computed with kinetis based boards. I'll open a PR to fix this after this one.

Issues/PRs references

None

@aabadie aabadie added Area: tests Area: tests and testing framework Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 28, 2019
@aabadie aabadie requested a review from cladmi January 28, 2019 09:33
@cladmi
Copy link
Contributor

cladmi commented Jan 28, 2019

There are indeed only this test file in these tests

$ ls tests/cond_order/tests/
01-run.py
$ ls tests/pkg_cmsis-dsp/tests/
01-run.py
$ ls tests/pkg_lora-serialization/tests/
01-run.py

For reference, it was added by #9567 where the tests scripts area automatically found.

@cladmi cladmi added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines labels Jan 28, 2019
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 I tested the tests are still run. The tested procedure also works with the error in master and only one message with this PR.
The commits could reference the PR deprecating defining tests but I will not block for this as I mentioned it in the PR. Keep them if you want.

@aabadie I let you merge like this or with updated commits as you want.

@cladmi cladmi added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jan 28, 2019
@aabadie
Copy link
Contributor Author

aabadie commented Jan 28, 2019

I let you merge like this or with updated commits as you want.

Let's merge like this! Thanks for reviewing.

@aabadie aabadie merged commit 782b181 into RIOT-OS:master Jan 28, 2019
@aabadie aabadie deleted the pr/make/distclean_cleanup branch January 29, 2019 18:15
miri64 added a commit to miri64/RIOT that referenced this pull request Jan 29, 2019
@danpetry danpetry added this to the Release 2019.04 milestone Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants