Skip to content

Conversation

odow
Copy link
Member

@odow odow commented Jul 20, 2021

Part of #1398

@blegat I found a bug in getting a slack objective bridge with a constant. Are you okay if I redo the tests a bit more systematically (e.g., like this PR) rather than picking a few MOI.Test functions to run?

@odow odow added the Submodule: Bridges About the Bridges submodule label Jul 20, 2021
@odow odow requested a review from blegat July 20, 2021 01:01
@blegat
Copy link
Member

blegat commented Jul 20, 2021

I think it's good to have both. We can keep the tests that are calling MOI.Test, are they difficult to convert from DeprecatedTest?

@odow
Copy link
Member Author

odow commented Jul 20, 2021

I guess it's not obvious to me why or how some of the tests were chosen, or what they are testing. I don't think we need to run the full battery of MOI.Test for every bridge? They already take quite a long time to run...

@blegat
Copy link
Member

blegat commented Jul 20, 2021

I guess it's not obvious to me why or how some of the tests were chosen, or what they are testing.

If you write a custom test for the bridge then you don't exclude the possibility that both the implementation and the tests are wrong and that cancels out so that the test pass. At least if you reuse an existing test, you exclude that possibility :)

I don't think we need to run the full battery of MOI.Test for every bridge? They already take quite a long time to run...

No, we can just keep it the way it is. Run it with one or two tests as is currently done is good enough.

@odow
Copy link
Member Author

odow commented Jul 21, 2021

I added them back.

@odow odow merged commit 5087c93 into master Jul 22, 2021
@odow odow deleted the od/bridge-objective branch July 22, 2021 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Submodule: Bridges About the Bridges submodule
Development

Successfully merging this pull request may close these issues.

2 participants