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

Test for DB queries #127

Merged
merged 22 commits into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7ecaa53
Testing ContestQueries.java
umbernhard Jun 19, 2024
5a7788d
AdministratorQueries is now fully tested, though some code changed mi…
umbernhard Jun 19, 2024
f627c7d
revising pom.xml to include integration tests in maven
umbernhard Jun 20, 2024
7f275d7
make sure maven includes integration tests
umbernhard Jun 20, 2024
82e0348
Adding tests for BMIQuery, also fixing an edge case
umbernhard Jun 20, 2024
7422e9a
removing unneeded import
umbernhard Jun 20, 2024
78a3b99
Working on CVR query tests
umbernhard Jun 20, 2024
4ffc9e1
Still working on CVRQueries. There's a lot of stuff in here!
umbernhard Jun 20, 2024
65803d2
Making drops cascade to prevent weird side effects
umbernhard Jun 20, 2024
7e23559
Changing deleteForCounty to return a count
umbernhard Jun 21, 2024
ec8c22a
putting back a final descriptor
umbernhard Jun 21, 2024
15e9423
making some lambdas more clear, adding a TODO about unreachable code
umbernhard Jun 21, 2024
6d995c9
Calling CastVoteRecordQueriesTest good for now. Not 100% coverage, bu…
umbernhard Jun 21, 2024
ed31a9f
Fixing missing CASCADE statement
umbernhard Jun 21, 2024
bcb32a8
disabling big tests to see if it unborks GitHub
umbernhard Jun 21, 2024
7781cb8
Trying to see if a different os makes a difference
umbernhard Jun 21, 2024
448c373
nope, that wasn't it
umbernhard Jun 21, 2024
db25f12
somehow StartAuditRoundTest got reverted to non testcontainers?
umbernhard Jun 21, 2024
cd456dd
Moving to @michelleblom's abstraction for db setup
umbernhard Jun 22, 2024
48d079f
adding big tests back in since they weren't the problem
umbernhard Jun 22, 2024
940ac00
starting a stub for this for now, gonna need to write all the other t…
umbernhard Jun 22, 2024
c334d60
Disabling ComparisonAuditQueriesTest because it's unfinished
umbernhard Jun 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/eclipse-project/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<properties>
<compileSource>11</compileSource>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<corla.test.excludedGroups>integration</corla.test.excludedGroups>
<!--<corla.test.excludedGroups>integration</corla.test.excludedGroups>-->
<checkstyle.skip>true</checkstyle.skip>
<maven.javadoc.skip>true</maven.javadoc.skip>
<maven.javadoc.failOnError>false</maven.javadoc.failOnError>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public static Administrator byUsername(final String the_username) {
// if there's exactly one result, return that
if (query_results.size() == 1) {
result = query_results.get(0);
}
}
// TODO: if two admins have the same username, null gets returned. This seems like a bug?
} catch (final PersistenceException e) {
Main.LOGGER.error("could not query database for administrator");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ Find the batch(bmi) that would hold the sequence number given.
result = new HashSet<BallotManifestInfo>(query.getResultList());
} catch (final PersistenceException e) {
Copy link
Member

Choose a reason for hiding this comment

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

I think e is still final.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ope, I missed it. Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Main.LOGGER.error("Exception when reading ballot manifests from database: ", e);
return Optional.empty();
}
return result.stream().findFirst();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public static OptionalLong countMatching(final RecordType the_type) {
} catch (final PersistenceException e) {
Main.LOGGER.error(COULD_NOT_QUERY_DATABASE);
}
if (result == null) {
if (result == OptionalLong.empty()) {
Main.LOGGER.debug("found no CVRs for type " + the_type);
} else {
Main.LOGGER.debug("query succeeded, returning CVR stream");
Expand Down Expand Up @@ -222,7 +222,7 @@ public static OptionalLong countMatching(final Long the_county, final RecordType
} catch (final PersistenceException e) {
Main.LOGGER.error(COULD_NOT_QUERY_DATABASE);
}
if (result == null) {
if (result == OptionalLong.empty()) {
Main.LOGGER.debug("found no CVRs for county " + the_county + ", type " + the_type);
} else {
Main.LOGGER.debug("query succeeded, returning CVR stream");
Expand Down Expand Up @@ -323,6 +323,7 @@ public static CastVoteRecord get(final Long the_county_id, final RecordType the_
final TypedQuery<CastVoteRecord> query = s.createQuery(cq);
final List<CastVoteRecord> query_results = query.getResultList();
// if there's exactly one result, return that
// TODO the else branch here shouldn't return null?
if (query_results.size() == 1) {
result = query_results.get(0);
}
Expand Down Expand Up @@ -352,6 +353,9 @@ public static CastVoteRecord get(final Long the_county_id, final RecordType the_
public static Map<Integer, CastVoteRecord> get(final Long the_county_id,
final RecordType the_type,
final List<Integer> the_sequence_numbers) {

// TODO: this doesn't handle the case where two CVRs in the same county have the same sequence number.
// TODO: There should probably be a DB error or something if that happens.
Map<Integer, CastVoteRecord> result = null;
final Set<Integer> unique_numbers = new HashSet<>(the_sequence_numbers);

Expand Down Expand Up @@ -411,7 +415,7 @@ public static List<CastVoteRecord> get(final List<Long> the_ids) {
} catch (final PersistenceException e) {
Main.LOGGER.error(COULD_NOT_QUERY_DATABASE);
}
if (result == null) {
if (result.isEmpty()) {
Main.LOGGER.debug("found no CVRs with ids " + the_ids);
return new ArrayList<>();
} else {
Expand Down Expand Up @@ -474,7 +478,7 @@ public static List<CastVoteRecord> atPosition(final List<Tribute> tributes) {
return t.getUri();
}))
// is it faster to let the db do this with an except query?
.filter(t -> !foundUris.contains(t.getUri())).map(t -> phantomRecord(t))
.filter(t -> !foundUris.contains(t.getUri())).map(CastVoteRecordQueries::phantomRecord)
.map(Persistence::persist).collect(Collectors.toSet());

results.addAll(phantomRecords);
Expand All @@ -496,8 +500,10 @@ public static List<CastVoteRecord> atPosition(final List<Tribute> tributes) {
}

final List<CastVoteRecord> returnList =
randomOrder.stream().filter(cvr -> null != cvr).collect(Collectors.toList());
randomOrder.stream().filter(Objects::nonNull).collect(Collectors.toList());
if (returnList.size() != uris.size()) {
// TODO: I'm pretty sure this code is unreachable, since any time |URIs| < |return|, we
// TODO: make phantoms until they equal. Maybe take this out?
// we got a problem here
Main.LOGGER
.error("something went wrong with atPosition - returnList.size() != uris.size()");
Expand Down Expand Up @@ -570,13 +576,15 @@ public static Long maxRevision(final CastVoteRecord cvr) {
q.setLong("countyId", cvr.countyID());
q.setString("imprintedId", cvr.imprintedID());

final Long result = (Long) q.getSingleResult();

if (null == result) {
try {
return (Long) q.getSingleResult();
} catch (final PersistenceException e) {
// the DB had a problem!
// TODO: Technically this should probably be an error code?
// TODO: Otherwise there's no way to discern this from a CVR with no revisions?
return 0L;
} else {
return result;
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,21 @@ public static Set<Contest> forCounty(final County the_county) {
/**
* Deletes all the contests for the county with the specified ID.
*
* @param the_id The county ID.
* @param the_county_id The county ID.
* @return the number of contests that were deleted, or -1 if an error occured.
*/
public static void deleteForCounty(final Long the_county_id) {
public static int deleteForCounty(final Long the_county_id) {
final Set<Contest> contests =
forCounty(Persistence.getByID(the_county_id, County.class));

int retval = -1;
if (contests != null) {
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are completely right, it definitely can. I even have a test for it higher up! 🤦

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

retval = contests.size();
for (final Contest c : contests) {
Persistence.delete(c);
}
}
Persistence.flush();
return retval;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,56 +31,15 @@
import org.hibernate.Session;
import us.freeandfair.corla.query.CastVoteRecordQueries;
import us.freeandfair.corla.query.Setup;
import us.freeandfair.corla.util.TestClassWithDatabase;

@Test(groups = {"integration"})
public class BallotSelectionTest {
public class BallotSelectionTest extends TestClassWithDatabase {

private BallotSelectionTest() {};

private Boolean return_cvr = true;

/**
* Container for the mock-up database.
*/
static PostgreSQLContainer<?> postgres
= new PostgreSQLContainer<>("postgres:15-alpine")
// None of these actually have to be the same as the real database (except its name), but this
// makes it easy to match the setup scripts.
.withDatabaseName("corla")
.withUsername("corlaadmin")
.withPassword("corlasecret")
// .withInitScripts("corlaInit.sql","contest.sql");
.withInitScript("SQL/corlaInitEmpty.sql");

@BeforeClass
public static void beforeAll() {
postgres.start();
Properties hibernateProperties = new Properties();
hibernateProperties.setProperty("hibernate.driver", "org.postgresql.Driver");
hibernateProperties.setProperty("hibernate.url", postgres.getJdbcUrl());
hibernateProperties.setProperty("hibernate.user", postgres.getUsername());
hibernateProperties.setProperty("hibernate.pass", postgres.getPassword());
hibernateProperties.setProperty("hibernate.dialect", "org.hibernate.dialect.PostgreSQL9Dialect");
Persistence.setProperties(hibernateProperties);
Persistence.beginTransaction();

}

@AfterClass
public static void afterAll() {
postgres.stop();
}

@BeforeMethod
public static void beforeEach() {
Persistence.beginTransaction();
}

@AfterMethod
public static void afterEach() {
Persistence.rollbackTransaction();
}

@Test()
public void testAuditedPrefixLengthWithNone() {
List<Long> cvrIds = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,23 @@

import static org.testng.Assert.assertEquals;

import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testng.annotations.*;

import us.freeandfair.corla.asm.CountyDashboardASM;
import us.freeandfair.corla.model.County;
import us.freeandfair.corla.model.CountyDashboard;
import us.freeandfair.corla.persistence.Persistence;
import us.freeandfair.corla.query.Setup;
import us.freeandfair.corla.util.TestClassWithDatabase;

import java.util.Properties;

@Test(groups = {"integration"})
public class StartAuditRoundTest {
public class StartAuditRoundTest extends TestClassWithDatabase {

private StartAuditRoundTest() {};


@BeforeTest()
public void setUp() {
Setup.setProperties();
Persistence.beginTransaction();
}

@AfterTest()
public void tearDown() {
try {
Persistence.rollbackTransaction();
} catch (Exception e) {
}
}

// this test doesn't do much yet
@Test()
public void testReadyToStartFalse() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package us.freeandfair.corla.query;

import org.testcontainers.containers.PostgreSQLContainer;
import org.testng.annotations.*;
import us.freeandfair.corla.model.Administrator;
import us.freeandfair.corla.model.County;
import us.freeandfair.corla.persistence.Persistence;
import us.freeandfair.corla.util.TestClassWithDatabase;

import java.util.Properties;

import static org.testng.AssertJUnit.assertEquals;
import static org.testng.AssertJUnit.assertNull;

@Test
public class AdministratorQueriesTest extends TestClassWithDatabase {

@Test
public void testByUsernameState() {

Administrator admin = new Administrator("testname",
Administrator.AdministratorType.STATE,
"fulltestname",
null);

Persistence.saveOrUpdate(admin);
assertEquals(AdministratorQueries.byUsername("testname"), admin);
}


@Test
public void testByUsernameCounty() {

County county = new County("test", 1L);
Persistence.saveOrUpdate(county);
Administrator admin = new Administrator("testname",
Administrator.AdministratorType.COUNTY,
"fulltestname",
county
);

Persistence.saveOrUpdate(admin);
assertEquals(AdministratorQueries.byUsername("testname"), admin);
}

@Test
public void testByUsernameStateAndCounty() {
County county = new County("test", 1L);
Persistence.saveOrUpdate(county);
Administrator countyadmin = new Administrator("county",
Administrator.AdministratorType.COUNTY,
"countyfull",
county);

Persistence.saveOrUpdate(countyadmin);

Administrator stateadmin = new Administrator("state",
Administrator.AdministratorType.STATE,
"statefull",
null);

Persistence.saveOrUpdate(stateadmin);

assertEquals(AdministratorQueries.byUsername("county"),countyadmin);
assertEquals(AdministratorQueries.byUsername("state"), stateadmin);
}

@Test
public void testSameName() {

Administrator first = new Administrator("state",
Administrator.AdministratorType.STATE,
"first",
null);

Administrator second = new Administrator("state",
Administrator.AdministratorType.STATE,
"second",
null);
Persistence.saveOrUpdate(first);
Persistence.saveOrUpdate(second);

assertNull(AdministratorQueries.byUsername("state"));
}

@Test
public void testDBError() {
// Close the DB
Persistence.commitTransaction();
assertNull(AdministratorQueries.byUsername("username"));
Persistence.beginTransaction();
}
}
Loading
Loading