Fix some more failing integration tests #707
Conversation
PetaPoco.Tests.Integration/Databases/OracleTests/OracleStoredProcTests.cs
Show resolved
Hide resolved
|
For future reference (referring to commit d97066b) when syncing private feature branches like this one, prefer a rebase with force-push over a merge to cut down on the extra commits in the history. Contributor docs don't specifically say anything about it yet (that's on me), so figured I'd mention it. In the web UI, it would need selected from the "Update branch" button dropdown. |
There was a problem hiding this comment.
Everything looks honky-dory. Major step toward the finish line with Oracle' support thanks to @Curlack. I've pulled the changes locally to run the entire test suite with all dbms's, and will merge this into the base feature branch provided no regressions from the core changes.
[EDIT] This drops failing Oracle tests down from 70/301 to 15/301. Nicely done @Curlack! Also, MariaDB, Postgres and SqlServer tests all pass with the changes; don't foresee any issues from the remaining ones either, but will let the runner finish before closing this.
Ok now this is a teaching moment for me. I saw the "Update Branch" button, but wasn't sure if I should press it. Famous last words: "Hmm, what does this button do?" Hahaha. I'm also beginning to realize that "rebase" has a different meaning than what I thought it means. I come from TFS, Azure Devs Ops and SourceSafe world. Not too many difference between them, but huge difference compared to Github. Also used SubVersion for a little while some years ago. Iirc that's closer to Github than those others. Anyway, I thought "rebase" means to move feature branch to different main branch. I can still see the button, should I go push it? 😆 |
|
@Curlack Don't push "Update branch" -- click the down arrow next to it and choose "Update with rebase". |
Ah, I see! So once it reads "Rebase branch" I should push it? |
Add unit test to confirm.
1. Try to drop the user first so we can use the "user still connected" failure to bypass setup i.e. assume already setup correctly. This is to allow the developer to stay connected to the database (via SQLPlus or SQL Developer). Modify OracleTestProvider 1. If Oracle setup fails, propagate failure to all tests without retrying for every test in the batch. This makes all tests fail with the same exception. In the case the user is still connected, setup is bypassed. Fix Oracle build scripts 1. Stored procedures don't use any characters to denote a parameter, so they have to be fully qualified e.g. ProcName.paramName to differentiate them from column names (if the same name is used e.g. age). Fix breaking tests: 1. QueryTests.cs had couple of malformed sql statements after string concatenation (missing space). 2. Override all (Oracle)StoredProcTests.cs methods that require an additional RefCursor output parameter. 3. Override all (Oracle)QueryTests.cs paging methods that require '*' to be aliased. 4. OracleQueryTests.cs had a couple of statements ending in a semi-colon, which it didn't appreciate. 5. Fix top 1 query in "QueryMultiple ForSingleResultsSetWithMultiPoco ShouldReturnValidPocoCollection". Also provide version 12c and above syntax. 6. Skip MultiResultSetWithMultiPoco tests since Oracle also does not support it (as with MSAccess and Firebird).
…ds. Oracle doesn't utilize the parameter prefix in this case. Fixes #691 Give OracleDatabaseProvider its own implementation of BuildPageQuery and use "Select null from dual" instead of "Select null" when ORDER BY clause is absent. Alo include version 12c and above syntax in a comment for future reference.
Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>
…of "undoing" the pramPrefix modification in OracleDatabaseProvider.
Co-authored-by: Stelio Kontos <37424493+Ste1io@users.noreply.github.com>
|
Whooohooo! I did it and everything is still green. |
Correct. Then when you're at your home PC, you can just pull your private feature branch from remote to sync up, and you'll see the additional commits made here locally also, sans the additional "changes merged" commit that is created by a merge. In this case, you'll still see one of those merge commits from the previous one I mentioned, but if you follow this in the future it will keep the git history much cleaner. With git, rebasing combines the commits into a single line with the commits appearing together, as supposed to two adding a merge commit that joins two separate lines (if you were looking at a git graph). Since it does perform a rewrite of history in some cases, it requires a force push afterwards. On private branches, such as this one, that's fine. On shared branches, such as the base feature branch this PR is targeting, a merge is safer and clearly shows how the private branches converge with their associated contributions. Edit: Actually, that merge commit seems to be gone now. I guess GitHub essentially reset to the first commit, and reapplied them all using rebase instead of merge? Cool...lol. They didn't have that fancy bit around when @asherber schooled me on this LMAO. |
The fork that is gone would be referring to Notice the gray "merge branch..." commit, which was created when merging as supposed to rebasing, and how there's a diversion from the branch that gets merged in:
Using rebase, all the commits are still there, but in a single straight line on your branch:
The lower half of the above image is from your previous commits in the last PR. The upper half is from this one using rebase instead of merge. |
|
Thanks @Ste1io . Very well explained. I get what rebase, in Github terms, means now. |
net472Tests in group: 3472 Outcomes netcoreapp3.1Tests in group: 3186 Outcomes All 15 failures are expected, and originating from Oracle. |
|
Nearly forgot...
|
Would've like to separate the core changes into a different PR, but got ahead of myself.