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

Remove top-level tests and test data #183

Merged
merged 4 commits into from
Aug 1, 2023

Conversation

sajith
Copy link
Member

@sajith sajith commented Jul 27, 2023

Issue is #182. Changes:

  • Removes the top-level tests/test_Solver.py, because it really is a test of PCE. I will move that test to pce (or at least parts of it that do not overlap with the existing pce tests). The issue to track this is Move some sdx-controller tests here pce#132.
  • Remove associated test data.
  • Remove top level samples directory, because the topology files are broken.

Also see #168.

@coveralls
Copy link

coveralls commented Jul 27, 2023

Coverage Status

coverage: 36.146%. remained the same when pulling 62176eb on 182.consolidate-tests-and-topologies into 3c3fbf7 on main.

@YufengXin
Copy link
Collaborator

These tests are largely the same as those in PCE. The purpose originally was to let Cong to test the PCE APIs within the SDX-Controller before integrating into the controller code in confidence. So it's Cong's call @congwang09.

@sajith
Copy link
Member Author

sajith commented Aug 1, 2023

That makes sense as an interim measure. However if we keep the tests here, it is going to duplicate the code that's been already moved to pce (in atlanticwave-sdx/pce#134). That is less than ideal.

If we really don't mind duplicating tests, we should still remove the static JSON files from here and use the ones from pce once atlanticwave-sdx/pce#139 is merged. I will update this PR based on Cong's feedback.

@congwang09
Copy link
Contributor

I think this is fine. I'll use the tests in pce for testing. And we can avoid. inconsistency between the two repo.

@sajith sajith merged commit d282c37 into main Aug 1, 2023
5 checks passed
@sajith sajith deleted the 182.consolidate-tests-and-topologies branch August 1, 2023 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Move top-level tests and test data to pce Gather topology/request samples and tests in one place
4 participants