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

Feature/performance test #749

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

seanmcguire12
Copy link

This PR creates performance tests for evaluating the various replay strategies against a constant (recording(s) in the database). Work in progress.

Summary

This PR addresses #704

Checklist

  • My code follows the style guidelines of OpenAdapt
  • I have performed a self-review of my code
  • If applicable, I have added tests to prove my fix is functional/effective
  • I have linted my code locally prior to submission
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
  • New and existing unit tests pass locally with my changes

How can your code be run and tested?

This code can be run with pytest -s /OpenAdapt/tests/openadapt/test_performance.py

Copy link
Member

@abrichr abrichr left a comment

Choose a reason for hiding this comment

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

Thank you for the great start @seanmcguire12 👍

Please let me know if you have any questions 🙏

openadapt/db/crud.py Show resolved Hide resolved
scripts/generate_db_fixtures.py Show resolved Hide resolved

return data

def format_sql_insert(table_name, rows):
Copy link
Member

@abrichr abrichr Jun 13, 2024

Choose a reason for hiding this comment

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

Please add types.

Also, what do you think about something like this:

from sqlalchemy import insert, Table, MetaData
from sqlalchemy.engine import Engine  # Assuming you have an engine instance

def generate_sql_insert(engine: Engine, table_name: str, rows: list) -> str:
    """Generates a SQL INSERT statement using SQLAlchemy for given rows and table.

    Args:
        engine (Engine): SQLAlchemy Engine connected to the database.
        table_name (str): Name of the table to insert data into.
        rows (list): List of dictionaries representing the rows to insert.

    Returns:
        str: A string representation of the SQL INSERT statement suitable for fixtures.sql.
    """
    metadata = MetaData(bind=engine)
    table = Table(table_name, metadata, autoload_with=engine)

    stmt = insert(table).values(rows)
    compiled = stmt.compile(engine, compile_kwargs={"literal_binds": True})
    return str(compiled) + ";"

Please fill out the type for rows, e.g. list[dict], list[BaseModelType] , list[db.Base], or similar.


if data["memory_stats"]:
file.write("-- Insert sample memory_stats\n")
file.write(format_sql_insert("memory_stat", data["memory_stats"]))
Copy link
Member

@abrichr abrichr Jun 13, 2024

Choose a reason for hiding this comment

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

How about something more DRY, e.g.:

rows_by_table_name = fetch_data(session)
for table_name, rows in rows_by_table_name.items():
    if not rows:
        logger.warning(f"no rows for table_name=}")
        continue
    with open(file_path, "a") as file:
        logger.info(f"writing {len(rows)=} to {file_path=} for {table_name=}")
        file.write(f"-- Insert sample rows for {table_name}\n")
        file.write(format_sql_insert(table_name, rows))

@abrichr
Copy link
Member

abrichr commented Jun 14, 2024

Related: #707

seanmcguire12 and others added 4 commits June 17, 2024 13:50
…ctions for interacting with the macos accessibility tree.
…st is as we expect. Still to do: add conditional statements to ensure that the tests run correctly based on the users OS.
…r is not closed during the recording (so that we can run tests on the final state).


logger.info(f"Value match: '{element_value}' == '{expected_value}'")
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about:

assert actual_value == expected_value, f"{actual_value=} {expected_value=}"

@abrichr
Copy link
Member

abrichr commented Jun 18, 2024

Looks great so far, thank you @seanmcguire12 ! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants