Skip to content

Add Builder Test Pytest Parser - #73

Merged
vmilosevic merged 18 commits into
tenstorrent:mainfrom
ctodTT:ctod/builder-tests
Sep 15, 2025
Merged

vmilosevic merged 18 commits into
tenstorrent:mainfrom
ctodTT:ctod/builder-tests

Conversation

@ctodTT

@ctodTT ctodTT commented Aug 8, 2025 •

Copy link
Copy Markdown
Contributor

This change adds a secondary parser for pytest JUnit XML files specifically created by the tt-mlir/test/python/golden/ tests. This is part of a broader effort to get the results of these tests into Superset, and the collect_data action is a key part of that pipeline.

Changes

  • The OpTests pydantic model has been modified to fit the one in data-airflow/dags/pipelines/ml_kernel_op_test/pydantic_models.py This is to ensure a common schema across the sftp call that the collect_data action performs
  • A new parser specific to the bespoke rich test result schema defined by tt-mlir/test/python/golden/conftest.py. It contains information about input tensors, hardware, git sha, and any other parameters that the test is run with.
  • A new filter to parser selection has been added that will pass reports that are both from main and part of a job with "builder" in the name to the BuilderPytestParser.

@ctodTT
ctodTT force-pushed the ctod/builder-tests branch from f27ca1b to 27002a9 Compare August 8, 2025 23:54

@vmilosevic vmilosevic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Parser Registration

Issue: Parsers registered with test_parser.py produce pydantic_models.Test objects, but your parser is producing pydantic_models.OpTest objects, which have a separate collection path and are uploaded in separate files.

Data Flow Paths:

pydantic_models.Test path:
generate_data → create_pipeline_json → create_cicd_json_for_data_analysis → ...

pydantic_models.OpTest path:
generate_data → create_optest_json → create_optest_reports

Solution: Register your parser in optests.py instead of test_parser.py

Testing

This project has multiple contributors, and the only way to ensure that others won't break the changes you made is to add regression tests. Once changes are merged to main, all pipelines start using the new collection code, so it's easy not to notice that something got broken.

Required Test Cases:

Unit tests for builder_pytest_parser.py

Integration tests: Add sample data files to collect_data/test/data/ and add test cases that try to read the actual files, similar to the test here

@ctodTT
ctodTT marked this pull request as ready for review September 3, 2025 14:54
jbakerTT and others added 8 commits September 3, 2025 14:55
* Add debugging code, re-enable the workflow to figure out what is working and what isn't.

* Update .github/workflows/on-community-issue.yml

Remove potential secrets leak

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fixed missing newline

* Changed output to only put out specific items.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ctodTT
ctodTT force-pushed the ctod/builder-tests branch from 66a2753 to 7286e57 Compare September 3, 2025 14:55
Comment thread .github/actions/collect_data/src/test_parser.py Outdated
Comment thread .github/actions/collect_data/src/cicd.py Outdated
parser = TTTorchModelTestsParser()

# Load job information to get job names
github_jobs_json_filename = f"{workflow_outputs_dir}/{pipeline.github_pipeline_id}/workflow_jobs.json"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe pipeline object already contains job information you need

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

job_id_to_name = {j.github_job_id:j.name for j in pipeline.jobs}

job_name = job_id_to_name.get(github_job_id, "")

# Select parser based on job name and git branch
if should_use_builder_pytest_parser(job_name, git_branch):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this exclusive or can both parsers extract data from same files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Technically both could extract data from the same file, but we only want this data to specifically be moved through the builder pytest parser pipeline. The other parser will currently fail due to the suite-wide properties I define to mark this as a builder test artifact.

Comment thread .github/actions/collect_data/src/test_parser.py Outdated
@ctodTT
ctodTT force-pushed the ctod/builder-tests branch 2 times, most recently from f56caca to 5427bb4 Compare September 15, 2025 19:53
@vmilosevic
vmilosevic merged commit cfaac33 into tenstorrent:main Sep 15, 2025
2 checks passed
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.

3 participants