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

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

Conversation

kanakOS01
Copy link

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
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).

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.

test: mismatch command line
2 participants