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
Fix remaining failing integration tests #710
Fix remaining failing integration tests #710
Conversation
I know this PR is only going to an internal branch right now, not to develop, but I think the changes to |
I can make the changes on develop and create a PR. How would you like to handle the changes in this PR though, first merge into develop and then pull into oracle-support? |
The |
Also, sorry to nitpick, but the first line of a git commit message should just be a brief (50 char) summary of what the commit does; the rest can follow after a blank line. This is because many git clients by default only display the first line of the message. See https://cbea.ms/git-commit/ for some good guidelines. |
I'm just sitting down to catch up on everything, been a hectic few days, but happy to see momentum is continuing at such a good clip. |
@Curlack, I'm preparing a discussion post I'll be posting here in the next few hours that directly addresses quoting behavior and identifier handling. Just wanted to let you know to avoid any possible repeated work and ensure we're on the same page in how it is handled. It's taken longer than I planned due to my absence, but also several other nuances came to the surface yesterday that factor in as well, that haven't been brought up and if not discussed now, will result in a lot of backtracking and recoding. |
I've pushed the last changes to the remaining failing oracle tests. These tests will only pass once we have the |
Wanting to flex my new skills...any objections to me squashing these commits into one? |
Go for it! Note that it's not always appropriate to squash commits. A commit should be a unit of work that has some significance. You might start out with a branch that has 6 or 7 commits, because you're following the maxim of committing often, and you might decide that really you've done two distinct things on that branch -- so you can squash down to two commits rather than one. |
Understood. Thanks. |
Also, you shouldn't squash or otherwise change commits that have already been merged somewhere else. One usual use case is to make local commits and squash them before pushing. Another, like this, is to have a series of commits in a pull request that get squashed before merging. GitHub can actually do that itself, from the web interface, but as the committer you have more control if you do it yourself. |
32d1ba1
to
78e040b
Compare
78e040b
to
f994020
Compare
#### IntegrationTests Split Oracle DB scripts, connection strings and tests in two. One set of tests to use PP OOTB (Delimited) and one set of tests to use PP with Custom provider and mapper (Ordinary). Fix Oracle setup script. Wait for tablespace drop operation to complete before creating the new tablespace. #### Core **Database.cs** - Escape Oracle Stored Procedure parameter names instead of ensuring paramater prefix. **OracleDatabaseProvider.cs** - Quote identifier when escaping (no Regex or ToUpperInvariant). - Disable statement caching for Oracle client.
f994020
to
5c875df
Compare
@Curlack is this PR ready to merge? |
I think so. Not sure if @Ste1io has some more changes. |
@pleb @asherber @Curlack, I apologize for my extended absence over the holidays, and appreciate all of your patience. @Curlack, I've merged the changes in the Following that, you'll need to pull the changes to your local from your origin (since your origin is what is being rebased using the update branch button. I'll follow up this comment with another addressing this PR specifically, but wanted to get our feature branch up to date with |
Since there no conflicts between @Curlack 's branch and the base branch, I would recommend not rebasing, unless there's reason to think that recent changes in cc: @Ste1io |
Apologies, I missed this one. |
Test Oracle Delimited and Ordinary Identifiers Separately.
IntegrationTests
Split Oracle DB scripts, connection strings and tests in two.
One set of tests to use PP OOTB (Delimited) and one set of tests to use PP with Custom provider and mapper (Ordinary).
Fix Oracle setup script. Wait for tablespace drop operation to complete before creating the new tablespace.
Core
Database.cs
OracleDatabaseProvider.cs