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

[NDB_Factory] Remove testing mode #7199

Merged
merged 23 commits into from
Jan 7, 2021
Merged

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Dec 14, 2020

The "Testing" mode in NDB_Factory complicates our code
and frequently confuses phan. It appears to not be used
by our testing suite, and the test suite often explicitly
sets it to not-test mode in order to get real connections,
and uses setDatabase/setConfig/etc to inject mocks rather
than using the factory's internal test mode. This removes
the mode in order to simplify our code.

The "Testing" mode in NDB_Factory complicates our code
and frequently confuses phan. It appears to not be used
by our testing suite, and the test suite often explicitly
sets it to not-test mode in order to get real connections,
and uses setDatabase/setConfig/etc to inject mocks rather
than using the factory's internal test mode. This removes
the mode and simplifies our code.
@driusan driusan added the State: Needs work PR awaiting additional work by the author to proceed label Dec 14, 2020
@driusan driusan added the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Dec 14, 2020
@kongtiaowang
Copy link
Contributor

kongtiaowang commented Dec 16, 2020

@driusan This commit should be helpful.
kongtiaowang@937eab9

@driusan driusan force-pushed the RemoveFactoryTestingMode branch from 19608db to 3bf3057 Compare December 16, 2020 14:50
@driusan driusan force-pushed the RemoveFactoryTestingMode branch from 6bd673c to 81deef5 Compare December 16, 2020 15:21
@driusan driusan removed "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help State: Needs work PR awaiting additional work by the author to proceed labels Dec 16, 2020
@driusan driusan added Cleanup PR or issue introducing/requiring at least one clean-up operation Meta PR does something that organizes, upgrades, or manages the functionality of the codebase labels Dec 16, 2020
@driusan driusan marked this pull request as ready for review December 16, 2020 16:12
@driusan
Copy link
Collaborator Author

driusan commented Dec 16, 2020

@kongtiaowang Now that this is passing Travis can you review? It's blocking something else that I was trying to do that I couldn't get phan to pass because of the (previous) convoluted return types in different paths.

@driusan
Copy link
Collaborator Author

driusan commented Dec 21, 2020

@kongtiaowang can you please review this now that it's passing Travis. It's blocking other work.

@driusan
Copy link
Collaborator Author

driusan commented Jan 4, 2021

@kongtiaowang This is still preventing important work on LORIS from going forward. Please review.

Copy link
Contributor

@kongtiaowang kongtiaowang left a comment

Choose a reason for hiding this comment

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

I have no problem with this PR.
LGTM.

@driusan driusan merged commit 0a17877 into aces:main Jan 7, 2021
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
The "Testing" mode in NDB_Factory complicates our code
and frequently confuses phan. It appears to not be used
by our testing suite, and the test suite often explicitly
sets it to not-test mode in order to get real connections,
and uses setDatabase/setConfig/etc to inject mocks rather
than using the factory's internal test mode. This removes
the mode and simplifies our code.
@ridz1208 ridz1208 added this to the 24.0.0 milestone Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup PR or issue introducing/requiring at least one clean-up operation Meta PR does something that organizes, upgrades, or manages the functionality of the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants