tests: add the slow tag for ros snapd integration test #1602

Merged
merged 1 commit into from Oct 18, 2017

Conversation

4 participants
Member

elopio commented Oct 10, 2017

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on bug reports?
  • If this is a new feature. Have you discussed the design on the forum?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh unit?

Requires #1603

kyrofa approved these changes Oct 10, 2017

This looks excellent. I do have one question: What if we have slow integration tests outside of the snapd suite? Should the slow_test attribute be moved up a class?

Member

elopio commented Oct 10, 2017

@kyrofa if that happens, we need to talk about it. The other suites shouldn't be slow, ideally. If we need it, we just move the var to the parent class and add it to the debian control.

Nice and simple. I like it!

@kalikiana kalikiana referenced this pull request Oct 13, 2017

Merged

ament plugin: new plugin #1583

6 of 6 tasks complete
Collaborator

sergiusens commented Oct 16, 2017

can someone re-trigger the failing test jobs?

Collaborator

kalikiana commented Oct 16, 2017

@sergiusens This won't pass on zesty without #1614.

ERROR: test_catkin_shared_ros.CatkinSharedRosTestCase.test_shared_ros_builds_without_catkin_in_underlay [...] apt.cache.FetchFailedException: W:GPG error: http://packages.ros.org/ros/ubuntu xenial InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 5523BAEEB01FA116, E:The repository 'http://packages.ros.org/ros/ubuntu xenial InRelease' is not signed. [...]

Member

kyrofa commented Oct 17, 2017

We're skipping instead, as in #1621, which has now landed. I just updated the branch, and will re-run the tests.

TESTING.md
+
+Some tests take too long. This affects the pull requests because we have to wait for a long time, and they will make Travis CI timeout because we have only 50 minutes per suite in there. The solution is to tag these tests as slow, and don't run them in all pull requests. These tests will only be run in autopkgtests.
+
+To mark a test as slow, set `slow_test = True` before the test `setUp`.
@sergiusens

sergiusens Oct 17, 2017

Collaborator

all implementations in here set it as a class attribute instead of in setUp. While working it is confusing and potentially error prone.

@elopio

elopio Oct 17, 2017

Member

I think it makes it less error prone to put it as a class attribute, because that assignment will always run before setUp.

From your comment, it's not clear what's your preference. I could document that the slow_test = True must be set in a class attribute, which is not totally correct, but could make it easier to understand.

looks good

@sergiusens sergiusens merged commit efb4900 into snapcore:master Oct 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sergiusens sergiusens added this to the 2.35 milestone Oct 18, 2017

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