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

Refactor algorithm tests to take transport parameter #208

Closed
wants to merge 6 commits into from

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Aug 14, 2019

Stack from ghstack:

This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

  • There are a series of small modifications included here as well,
    including more consistent variable naming, and a more consistent
    usage of test parameterization generator functions.
  • Because this commit touches many lines anyway, I also ran
    clang-format on all files under gloo/test to make the style
    consistent.

Differential Revision: D17072673

This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.
pietern added a commit that referenced this pull request Aug 14, 2019
This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.

ghstack-source-id: 46122ee90d31cd486e5fba23165d0b1d3f952a42
Pull Request resolved: #208
This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.
@pietern pietern mentioned this pull request Aug 26, 2019
This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.
This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.
This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.
gloo/test/allgather_test.cc Show resolved Hide resolved
This is required to run the same algorithm tests against multiple
transports. The test suite is comprised of both class algorithms and
function algorithms. The former require fixed buffers, which are only
supported on the tcp backend (and will be phased out). The latter
require the unbound buffers, which are supported on both the tcp and
uv (not yet merged) transports. This means we can't set the transport
globally and forget about it, since it depends on the test whether or
not it works with the selected transport.

Instead, by parameterizing the tests we allow for multiple transports
to be tested in the same test run. If a transport is not
available (e.g. not compiled) then those tests can be skipped.

Side notes:

* There are a series of small modifications included here as well,
  including more consistent variable naming, and a more consistent
  usage of test parameterization generator functions.
* Because this commit touches many lines anyway, I also ran
  clang-format on all files under `gloo/test` to make the style
  consistent.
@facebook-github-bot
Copy link

@pietern merged this pull request in 767c232.

@facebook-github-bot facebook-github-bot deleted the gh/pietern/7/head branch October 29, 2019 14:15
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.

3 participants