Test for DB queries #127
Test for DB queries #127
Conversation
| @@ -109,10 +109,8 @@ public static Set<Contest> forCounty(final County the_county) { | |||
| public static void deleteForCounty(final Long the_county_id) { | |||
| final Set<Contest> contests = | |||
| forCounty(Persistence.getByID(the_county_id, County.class)); | |||
| if (contests != null) { | |||
There was a problem hiding this comment.
I really like the idea of going through everything and changing it from something/null with null checks to Optional(something or nothing) with .ispresent checks. I'm just a little unsure about this particular case - are you sure you changed forCounty so that it can never return null?
There was a problem hiding this comment.
You are completely right, it definitely can. I even have a test for it higher up! 🤦
There was a problem hiding this comment.
I have now put this back, and also changed the function to return a count of how many contests it deletes so transient DB errors can be detected.
| @@ -204,8 +204,9 @@ Find the batch(bmi) that would hold the sequence number given. | |||
| cq.select(root).where(cb.and(disjuncts.toArray(new Predicate[disjuncts.size()]))); | |||
| final TypedQuery<BallotManifestInfo> query = s.createQuery(cq); | |||
| result = new HashSet<BallotManifestInfo>(query.getResultList()); | |||
| } catch (final PersistenceException e) { | |||
There was a problem hiding this comment.
ope, I missed it. Thanks!
| Persistence.beginTransaction(); | ||
|
|
There was a problem hiding this comment.
@michelleblom pointed out when I set all this up explicitly that there's probably a much slicker way to do it - possibly setting all these values in test/resources/test.properties.
This worked great in the raire-service: https://github.com/DemocracyDevelopers/raire-service/tree/main/src/test/resources
but we were not keen to fiddle with colorado-rla in case we messed up the other tests. But now that you've got the other tests working, possibly you/we can avoid doing all this postgresql container config explicitly every time.
There was a problem hiding this comment.
Ah thanks, that's a great idea.
umbernhard
force-pushed
the
queries-tests
branch
2 times, most recently
from
3feb685 to
f787fb0
Compare
|
I'm marking this as ready for review, I think I was taking a wrongheaded approach in bundling all these changes in 1 PR, going forward I will break each new/modified test file into its own PR to make reviews less of a nightmare. @vteague @michelleblom I can do that with this one too if that's easier for you than going through this one. |
…ests then come back for this one...
umbernhard
force-pushed
the
queries-tests
branch
from
3006bba to
940ac00
Compare
No description provided.