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

test: add test for mismatch package build #4785

Merged
merged 6 commits into from
Mar 4, 2025

Conversation

kanakOS01
Copy link
Contributor

Fixes #4681

Changes:

  • Updated test/test_mismatch_cli.py to add a test to check whether the mismatch package will be properly build and run or not.
  • Added build dependency in dev-requirements.txt which is required for building the package for testing.

@pytest.mark.skipif(not LONG_TESTS(), reason="Skipping long tests")
def test_package_build(build_cleanup):
print("Building package")
build = subprocess.run(["python", "-m", "build"], check=True)
Copy link
Contributor

@Prtm2110 Prtm2110 Feb 10, 2025

Choose a reason for hiding this comment

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

Hello, To ensure the code runs on all systems, consider using the path to the Python executable, which can be accessed via sys.executable and import sys. Other than this it looks good!

Suggested change
build = subprocess.run(["python", "-m", "build"], check=True)
build = subprocess.run([sys.executable, "-m", "build"], check=True)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't know about this. thanks.

build = subprocess.run(["python", "-m", "build"], check=True)
assert build.returncode == 0, "Python build failed"

print("Install package")
Copy link
Contributor

@Prtm2110 Prtm2110 Feb 10, 2025

Choose a reason for hiding this comment

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

Suggested change
print("Install package")

We should not have any print statements in a test file

@Prtm2110
Copy link
Contributor

Thank you writing tests for #4669 (referencing it here as well). just one more thing from my side build in dev-requirements can be moved to up (alphabetical order).

@kanakOS01
Copy link
Contributor Author

Thank you writing tests for #4669 (referencing it here as well). just one more thing from my side build in dev-requirements can be moved to up (alphabetical order).

the dev-requirements are not in alphabetical order anyways so i just put it at the end

@kanakOS01
Copy link
Contributor Author

Hi @terriko
Do I need to make some update to have the test pass or is this unrelated?

@terriko
Copy link
Contributor

terriko commented Feb 21, 2025

I'm going to update this branch because we had some difficulties with tests on python 3.10 and updated that job to use python 3.13. If it fails this time you may have to fix it, though!

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Looks like the tests fails are unrelated, so we're good to get this merged. Congrats on your first merged PR with us, @kanakOS01 !

@terriko terriko merged commit d3ac5f0 into intel:main Mar 4, 2025
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: mismatch command line
3 participants