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

Move tests to top-level #186

Merged
merged 12 commits into from
Jul 6, 2023
Merged

Move tests to top-level #186

merged 12 commits into from
Jul 6, 2023

Conversation

sajith
Copy link
Member

@sajith sajith commented Jun 30, 2023

Part of #184. Changes:

  • Moves fabrictestbed_extensions/tests/ to a top-level tests/ folder, and rearranges into a directory structure like so:
tests/
├── benchmarks
│   ├── gpu_tesla_t4_benchmark.py
│   ├── link_benchmark.py
│   ├── local_network_benchmark.py
│   ├── network_benchmark_tests.py
│   └── nvme_benchmark.py
├── integration
│   ├── abc_test.py
│   ├── component_tests.py
│   └── hello_fabric.py
└── unit
    ├── data
    │   └── dummy-token.json
    ├── __init__.py
    └── test_basic.py

The advantage of moving tests to top-level is that the wheel package will not have fabrictestbed_extensions.tests module. Top-level tests directory also won't be included in the wheel package. Tests are useful for developers of FABLib, but not so for the consumers of the wheel package.

  • Adds a tox.ini, onto which we can add environments for running integration tests and benchmarks. I am still figuring out how to make them runnable. I suppose they are figure-outtable.
  • The integration tests and benchmarks are not usable yet, and @kthare10 is of the opinion that they could simply be removed, and acceptance test notebooks could be re-purposed as FABlib integration tests. I am not so sure about removing the existing tests right now. I would like to try update them and make them useful, if possible, before abandoning them altogether. Either way I would like to do that in follow-up PRs after assessing their usefulness.

@sajith sajith self-assigned this Jun 30, 2023
@sajith sajith linked an issue Jun 30, 2023 that may be closed by this pull request
@sajith sajith marked this pull request as draft June 30, 2023 21:47
@coveralls
Copy link
Collaborator

coveralls commented Jun 30, 2023

Pull Request Test Coverage Report for Build 5447577478

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.0%) to 20.712%

Totals Coverage Status
Change from base Build 5298386964: 1.0%
Covered Lines: 957
Relevant Lines: 4181

💛 - Coveralls

@sajith sajith requested review from paul-ruth and kthare10 July 6, 2023 20:13
@sajith sajith marked this pull request as ready for review July 6, 2023 20:13
Copy link
Collaborator

@kthare10 kthare10 left a comment

Choose a reason for hiding this comment

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

+1

@sajith sajith merged commit d5f0d71 into main Jul 6, 2023
11 of 12 checks passed
@sajith sajith deleted the 184.update-tests branch July 6, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document/update integration tests
3 participants