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

Automate Conversion of Workbooks to IR JSON and Update Test Logic #4326

Closed
sambodeme opened this issue Sep 26, 2024 · 3 comments · Fixed by #4354
Closed

Automate Conversion of Workbooks to IR JSON and Update Test Logic #4326

sambodeme opened this issue Sep 26, 2024 · 3 comments · Fixed by #4354
Assignees
Labels

Comments

@sambodeme
Copy link
Contributor

Description:

Currently, the FAC application uses a set of workbooks for testing, which are stored in various locations:

  • Under the audit app
  • Under the historical_migration app
  • Inside folders named should_pass and should_fail.

The test logic reads each workbook file and converts it into a JSON format called the IR model, which is then used for validation checks and other test-related processes.

Task:

This task is to implement the following enhancements:

  1. Automate Workbook-to-IR Conversion:

    • Update the logic to build fixture workbooks or IR JSON files as needed, instead of only building workbooks.
    • Store the converted IR JSON files instead of the original workbook files.
  2. Update Test Logic:

    • Modify the current test logic to consume the pre-converted IR JSON files rather than converting the workbooks into IR during each test run.
    • Ensure the test logic processes the IR JSON files for validation checks and other related tasks.
@danswick
Copy link
Contributor

Modify the current test logic to consume the pre-converted IR JSON files rather than converting the workbooks into IR during each test run.

What do you think about moving this change to the validation step? If validation could (optionally) consume IR and skip the workbook parsing process entirely, that could open up pathways for consuming non-workbook data. I'm thinking of things like revising and re-disseminating reports (resubmission-ish), interactively editing workbook data (far future), and possibly even merging duplicate reports.

If something like this makes sense, the tests could specify either the IR pathway or the full workbook pathway depending on what we want to test.

@jadudm
Copy link
Contributor

jadudm commented Sep 26, 2024

FWIW, the Python validations only operate on the IR.

We convert the workbook to the IR, and that is the last time we look at the workbook. We apply a few cleanup transformations to the IR (in some cases), and then we do all of our validations on the IR. At the end, we convert the IR to the internal JSON object notation only because we wanted to keep the JSON Schema validation as a backstop.

But, I might not understand the question. We don't currently store the IR, which might stand in the way of some things.

@danswick
Copy link
Contributor

We convert the workbook to the IR, and that is the last time we look at the workbook.

Ah, ok that makes sense! To make my earlier comment more concise: what if, instead of tweaking the test logic to accept IR, we tweaked the intake logic to accept either a workbook or IR?

@sambodeme sambodeme self-assigned this Sep 27, 2024
@sambodeme sambodeme moved this from Triage to In Progress in FAC Sep 30, 2024
sambodeme added a commit that referenced this issue Oct 7, 2024
@sambodeme sambodeme linked a pull request Oct 7, 2024 that will close this issue
18 tasks
sambodeme added a commit that referenced this issue Oct 7, 2024
sambodeme added a commit that referenced this issue Oct 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 11, 2024
#4354)

* #4326 Converted workbook fixtures into json fixtures

* #4326 Updated logic to extract data from both xlsx and json files

* #4326 Updated test cases

* Added logic to convert xlsx files into json

* Linting

* #4326 Linting

* #4326 bug fix

* Updated fixture files to accomodate

* Removed dead code
@github-project-automation github-project-automation bot moved this from In Progress to Done in FAC Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants