Skip to content

Add test case for mlc doc script detect-os #511

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

shahfarazz
Copy link

This PR adds a basic test case for the mlc doc script command targeting the detect-os script. It checks if the README.md is correctly generated.

📌 Notes
The test checks for successful execution and file presence.

Verified locally using pytest.

Let me know if you'd like help improving or customizing this further!

@shahfarazz shahfarazz requested a review from a team as a code owner July 15, 2025 18:08
Copy link
Contributor

github-actions bot commented Jul 15, 2025

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@shahfarazz
Copy link
Author

I’ve signed and submitted the CLA, just waiting for approval. Let me know if anything else is needed from my side!

@anandhu-eng
Copy link
Contributor

anandhu-eng commented Jul 15, 2025

Hey @shahfarazz , thanks for dropping a PR.

Just wanted to ask how are we running the test. We usually have GitHub action workflow files for these tests, one such examples would be found here.

@arjunsuresh should we add the test for mlc doc script in mlcflowor mlperf-automations

Edit: I see that these tests would be useful if we include it in both mlcflow(in test-mlc-core-actions.yaml) and mlperf-automations(call the mlc-core-action workflow)

@shahfarazz
Copy link
Author

Thanks, @anandhu-eng! At the moment, I’ve just run the test locally with pytest to validate functionality. Once the CLA is cleared and I get confirmation on where to place the workflow (either here or in mlcflow), I’ll proceed to set up the GitHub Action accordingly.

Appreciate the feedback!

@arjunsuresh
Copy link
Collaborator

@anandhu-eng yes, we can place the github action in both the repos. May be in the doc action, we can return a key readme_path which makes it easier to do tests.

@shahfarazz
Copy link
Author

recheck

@shahfarazz
Copy link
Author

Hi, I received email confirmation this morning at 5:37 AM Central Time from Liz Walker that my CLA submission was approved. It still hasn’t updated on GitHub, so I’m waiting for that to reflect. Let me know if there’s anything else needed from my end!

Thanks,
Faraz

@anandhu-eng anandhu-eng mentioned this pull request Jul 17, 2025
1 task
@arjunsuresh
Copy link
Collaborator

Hi @shahfarazz thanks for your PR. The CLA approval shouldn't have taken more than a couple of days. It seems your github user is still not approved.

But adding a test for doc action for detect-os inside the detect-os script is not the proper way. Because that would imply adding a test for every action for every scripts which is not scalable. Instead if we add a github action for doc action which will automatically initiate for any change in the script meta, that'll be better. Actually the below action is doing that but it is missing the check for timestamp check which you have added in your PR. Do you think it is possible to apply it to the below GH action?

https://github.com/mlcommons/mlperf-automations/blob/main/.github/workflows/document-scripts.yml

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