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

TEST: update usb tests and guard them with macro #12498

Merged
merged 3 commits into from
Feb 27, 2020

Conversation

jamesbeyond
Copy link
Contributor

Summary of changes

Some feedback we received recently: the USB devices tests are quite different from the rest of the greentea tests.

  • They are not like other Greentea tests works straightaway
  • Silicon partners need to read through the README file and implement a series of local configurations before running the tests
  • 2 USB cables need to be physically plugged into to host, instead of one
  • There are some known limitations already some tests won't work with some platforms

So we decided to guard those tests with a macro. when silicon partners want to port the USB devices, he will need to follow the README instructions. and then run the tests.

Impact of changes

Migration actions required

Documentation

Updates included


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@MarceloSalazar @fkjagodzinski @maciejbocianski


@MarceloSalazar
Copy link

FYI @ARMmbed/team-st-mcd

@@ -863,3 +865,4 @@ void USBEndpointTester::start_ep_in_abort_test()
write_start(_endpoints[EP_INT_IN], _endpoint_buffs[EP_INT_IN], (*_endpoint_configs)[EP_INT_IN].max_packet);
}
#endif
#endif //USB_DEVICE_TESTS
Copy link
Contributor

Choose a reason for hiding this comment

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

Lack of new line at the file end, Applies to all files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@ciarmcom
Copy link
Member

@jamesbeyond, thank you for your changes.
@maciejbocianski @fkjagodzinski @MarceloSalazar @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Fix the missing new line at the end of the line.

Is this "exclude" the way we want to use? greentea does not support something similar to --exclude usb-* - wouldn't this be preferable ?

@mergify mergify bot dismissed 0xc0170’s stale review February 25, 2020 17:14

Pull request has been modified.

@jamesbeyond
Copy link
Contributor Author

jamesbeyond commented Feb 25, 2020

Fix the missing new line at the end of the line.

Fixed, Could you remind why we need this empty line? @0xc0170, I remember it is in the contributions requirement, but can't recall why

Is this "exclude" the way we want to use? greentea does not support something similar to --exclude usb-* - wouldn't this be preferable ?

Greentea has an excluding method. but it command line will end up too long if we excluding too many things.
Base on the feedback from @MarceloSalazar, I think partners turn to use greentea without any excluding options, which those tests end up failing because of local configurations. Also I agree that using the macro would be a benefit for both CI and partners

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 26, 2020

Fixed, Could you remind why we need this empty line? @0xc0170, I remember it is in the contributions requirement, but can't recall why

at least uvision prints a warning , and possibly some other but dont recall any other. We always keep one line empty since the beginning.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 26, 2020

CI started

@mergify mergify bot added needs: CI and removed needs: work labels Feb 26, 2020
@mbed-ci
Copy link

mbed-ci commented Feb 26, 2020

Test run: SUCCESS

Summary: 5 of 5 test jobs passed
Build number : 1
Build artifacts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants