-
Notifications
You must be signed in to change notification settings - Fork 438
DRAFT - Reset database UI testing data repeatability #1538
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
base: development
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| //Cypress test for resetDatabase.js - Angel43v3r | ||
|
|
||
| describe('Database Reset', () => { | ||
| it('should reset and repopulate the test database', () => { | ||
| cy.log('Starting resetDatabase...') | ||
| cy.task('resetDatabase').then((output: string) => { | ||
| cy.log(`Database Reset Output: ${output}`); | ||
| console.log('Full output:', output); | ||
| expect(output).to.include('Finished generating the test data'); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| //Author: ANGEL43V3R - added a script to reset the database and then repopulate it. | ||
| /** | ||
| * Author: ANGEL43V3R | ||
| * | ||
| * This script resets the test database (oed_testing) by dropping and recreating the test database. | ||
| * It then uses generateTestingData to populate the table using generateSine function. | ||
| */ | ||
|
|
||
| console.log('PGPASSWORD:', typeof process.env.PGPASSWORD, process.env.PGPASSWORD); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a draft but this will need to go when finalized
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the note, I'll make sure its cleaned up before submitting the final version |
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you help me understand how this differs and why from src/server/test/common.js that is used in testing to check/reset DB/etc?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi yes, I initially thought the same thing and tried to use the reset logic from common.js, but then I realized Cypress runs test differently. Mocha/Chai runs test in Node.js and can directly connect to or access the database using 'pg', but Cypress cannot and does not have direct access to the database. Because of that, I had to create resetDatabase.js to set up the reset and test data logic for Cypress. It still uses the same test database, oed_testing, so the main database is safe. It also follows the same idea as common.js where it wipes and repopulates the data for Mocha/Chai, I just adjusted it to work with Cypress.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the clarification. I think putting a comment in the code to explain this would be valuable so people will understand what is up. It can even link to the other code for the Mocha/Chai version. |
||
|
|
||
| const { Client } = require('pg'); | ||
| const { generateSine } = require('../data/generateTestingData'); | ||
|
|
||
| //Admin configuration to connect to the Postgres database | ||
| //NOTE: if want to run the test locally change host: 'database' to 'localhost' | ||
| const adminConfig = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this used to drop the user's DB information? If so, does this mean that running the Cypress tests would remove their data? I'm not sure but OED should try to avoid that.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this should not drop or affect the main database or their data, the resetDatabase.js is suppose to only wipe and repopulate the data inside the oed_testing.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given this, I recall there being this config and a test one. How do those relate and how are they used? I suspect comments in the code would clear this up.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In resetDatabase.js I am connecting to it using the |
||
| user: process.env.OED_DB_USER, | ||
| host: process.env.OED_DB_HOST, | ||
| database: 'postgres', | ||
| password: process.env.OED_DB_PASSWORD, | ||
| port: process.env.OED_DB_PORT, | ||
| }; | ||
|
|
||
| //Test configuration to connect to the test database (oed_testing) | ||
| //NOTE: if want to run the test locally change host: 'database' to 'localhost' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you help me understand what this means? OED usually runs on the local machine (for server) and client web browser. I have not tried this yet since I don't want to accidentally wipe my DB of other data.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry about that, I tend to write notes for myself during development and haven't cleaned up the code. This note I was just trying to mention that if I run the test inside Docker container then the database hostname need to be set to 'database' but if I want to run the tests directly on my local machine (not using docker) then I need to change the host from database to localhost. This comment is mostly a note to myself since I was still trying to learn how the tools work.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. I think the current directions on the design doc have people run the tests in the cypress Docker container. Do you think that should change?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I followed the documentation and it specifies to run the tests in the Cypress and Docker container so I assumed that's the intended approach. I think it's fine to continue running the tests this way. The only problem on my end was that I have to interact with the database to reset and repopulate it, which required a script, but other UI tests should work properly since they don't require direct access to the database and can operate through the apps UI. |
||
| const testDbConfig = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this relate to an item with a very similar name in src/server/test/common.js?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, just want to make sure I understand the question. Are you asking about how my resetDatabase.js relate to the common.js?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This object seems to be very similar to the one in the file I mentioned. Do both need to be defined or could one be reused?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi yes the logic in resetDatabase.js and common.js looks similar and they both reset and repopulate the test database. The difference is that common.js is designed to access the database directly using Node.js which works for Mocha/Chai but does not work for Cypress. Cypress runs in a browser and cannot access the database directly and so I made resetDatabase.js to work around that. This allows Cypress to call the database indirectly and pretty much mimics the logic in common.js. I am pretty sure that if you really want to use some of the logic in common.js that can be implemented, but I choose to do it separately because its cleaner and there is not much code that I found that I can reuse. |
||
| user: process.env.OED_DB_USER, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please change space indenting to tab indenting.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, will fix and update the indentation to use tabs instead of spaces. |
||
| host: process.env.OED_DB_HOST, | ||
| database: process.env.OED_DB_TEST_DATABASE, | ||
| password: process.env.OED_DB_PASSWORD, | ||
| port: process.env.OED_DB_PORT, | ||
| }; | ||
|
|
||
| /** | ||
| * Drops and recreates the oed_testing database | ||
| */ | ||
| async function resetTestDatabase() { | ||
| const client = new Client(adminConfig); | ||
| await client.connect(); | ||
|
|
||
| try { | ||
| console.log('Dropping test database (if exists)...') | ||
| await client.query('DROP DATABASE IF EXISTS oed_testing;'); | ||
|
|
||
| console.log('Recreating the test database...') | ||
| await client.query('CREATE DATABASE oed_testing WITH OWNER oed;'); | ||
| console.log('Test database reset successfully!'); | ||
| } catch (err) { | ||
| console.error('Error found during database reset: ', err); | ||
| throw err; | ||
| } finally { | ||
| await client.end(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Create table and insert generated test data | ||
| */ | ||
| async function insertTestData() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused as to what this will be used for and why a new table is needed. OED has a readings table for meter data and graphing. Is that what you are trying to do? If so, we can discuss how and OED provided ways of doing that. If not, then let me know what I am missing. |
||
| const client = new Client(testDbConfig); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering why the DB connection seems to differ from how it is done in general and for our mocha testing? |
||
| await client.connect(); | ||
|
|
||
| try { | ||
| //Create database tables | ||
| await client.query( | ||
| ` | ||
| CREATE TABLE IF NOT EXISTS sine_data ( | ||
| id SERIAL PRIMARY KEY, | ||
| value DOUBLE PRECISION NOT NULL, | ||
| start_timestamp TIMESTAMP NOT NULL, | ||
| end_timestamp TIMESTAMP NOT NULL | ||
| ); | ||
| ` | ||
| ); | ||
|
|
||
| // Generate sine data sample | ||
| console.log ('Generating the sine test data...') | ||
| const sineData = generateSine( | ||
| '2025-01-01 00:00:00', | ||
| '2025-01-02 00:00:00', | ||
| { | ||
| timeStep: {hour: 1 }, | ||
| maxAmplitude: 10 | ||
| } | ||
| ); | ||
|
|
||
| //Check if sine data is an array or if array is empty | ||
| if (!Array.isArray(sineData) || sineData.length === 0) { | ||
| throw new Error ('Invalid sine data, no sine data generated!') | ||
| } | ||
|
|
||
|
|
||
| //Insert generated data in rows | ||
| console.log (`Inserting ${sineData.length} rows of test data...`) | ||
| for (const row of sineData) { | ||
| await client.query( | ||
| 'INSERT INTO sine_data (value, start_timestamp, end_timestamp) VALUES ($1, $2, $3)', | ||
| [row.value, row.startTimeStamp, row.endTimeStamp] | ||
| ); | ||
| } | ||
|
|
||
| //Verifying by running a sample query | ||
| const res = await client.query('SELECT COUNT(*) FROM sine_data;'); | ||
| console.log('Inserted rows of sine data successfully! Total rows of sine data:', res.rows[0].count); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another console.log.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tend to use console.log during development to help verify things are running smoothly. I will be happy to remove it if necessary, just let me know what you prefer.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is fine to use console.log during your work. They need to be removed or use the OED logging system before the work is finalized.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where can I find the documentation for the OED logging system?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are doing UI testing, I should have said before that it is unusual to use the logging system. It would log to the testing DB that is not normally used. I would expect that most (or all) of the console.log msgs are for debugging and would go once the code is finalized. If you want to learn about the logging system, look at src/server/log.js. It uses src/server/models/LogMsg.js to actually store the log msg. src/server/routes/logs.js is the route for the server to accept new log msgs. I has not been used much on the client but see src/client/app/utils/api/LogsApi.ts, src/client/app/utils/api/index.ts & src/client/app/redux/actions/logs.ts for a start. Searching for logToServer from the last file will find uses on the client. I hope this helps and let me know if you want anything else. |
||
| } finally { | ||
| await client.end(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Main function - resets and repopulate the database | ||
| */ | ||
| async function main() { | ||
| try { | ||
| console.log('Starting database reset...') | ||
| await resetTestDatabase(); | ||
| console.log('Inserting test data...') | ||
| await insertTestData(); | ||
| console.log('Finished generating the test data!'); | ||
| } catch (err) { | ||
| console.error('Error during test setup:', err); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to think about how errors are handled for this. |
||
| } | ||
| process.exit(0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to exit with 0 whether it worked or not. Is that what is desired?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for pointing that out, the current setup does exit with 0 whether it worked or not, so this definitely needs to be corrected. Ill do an update on the script and make it exit with 0 only on success and then with 1 if any errors occurs. Will that work with you?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems fine. |
||
| } | ||
|
|
||
| main(); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra tabs on otherwise blank line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, I will clean up the extra tabs from the blank line before submitting the final.