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

Fix the rootdir in the nouse_install.yml #273

Merged
merged 9 commits into from
Feb 2, 2024
Merged

Conversation

ThreeMonth03
Copy link
Collaborator

Modify pytest --rootdir=/tmp -v to pytest --rootdir=. -v.

Copy link
Member

@yungyuc yungyuc left a comment

Choose a reason for hiding this comment

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

  • Make it explicit that --rootdir=. is a workaround.

@@ -83,7 +83,7 @@ jobs:
python3 -c 'import os; print(os.getcwd())'
python3 -c "import modmesh; print(modmesh.__file__)"
python3 -c "import _modmesh; print(_modmesh.__file__)"
pytest --rootdir=/tmp -v
pytest --rootdir=. -v
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the original code in a comment and add a link to the root cause. When the bug in pytest is fixed we can change it back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for your suggestions.
I write the comments, including the issue and the original code.

@@ -166,5 +166,5 @@ jobs:
python3 -c 'import os; print(os.getcwd())'
python3 -c "import modmesh; print(modmesh.__file__)"
python3 -c "import _modmesh; print(_modmesh.__file__)"
pytest --rootdir=/tmp -v
pytest --rootdir=. -v
Copy link
Member

Choose a reason for hiding this comment

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

Change both places.

Copy link
Collaborator Author

@ThreeMonth03 ThreeMonth03 left a comment

Choose a reason for hiding this comment

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

Here are the modifications.

Comment on lines 86 to 90
### The following command is the original commend, and it will fail on pytest == 8.0.0 .
### pytest --rootdir=/tmp -v
### Here is the issue and temporary solution: https://github.com/pytest-dev/pytest/issues/11781
### The alternative command to solve the issue is ```pytest --rootdir=. -v```.
pytest --rootdir=. -v
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I add the comment, including the original command, the modified command, and the source of the issue and solutions.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, the messages are clear. Could you please follow the comment style to use a single # symbol?

Copy link
Collaborator Author

@ThreeMonth03 ThreeMonth03 Feb 1, 2024

Choose a reason for hiding this comment

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

Thanks, the messages are clear. Could you please follow the comment style to use a single # symbol?

@yungyuc No problem, I modify the inconsistent part and push the modification to the branch.

Comment on lines 173 to 177
### The following command is the original commend, and it will fail on pytest == 8.0.0 .
### pytest --rootdir=/tmp -v
### Here is the issue and temporary solution: https://github.com/pytest-dev/pytest/issues/11781
### The alternative command to solve the issue is ```pytest --rootdir=. -v``` .
pytest --rootdir=. -v
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also add the comment here.

Copy link
Collaborator Author

@ThreeMonth03 ThreeMonth03 left a comment

Choose a reason for hiding this comment

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

I modify the number of # to keep the consistent coding style.

Comment on lines +86 to +89
# The following command is the original commend, and it will fail on pytest == 8.0.0 .
# pytest --rootdir=/tmp -v
# Here is the issue and temporary solution: https://github.com/pytest-dev/pytest/issues/11781
# The alternative command to solve the issue is ```pytest --rootdir=. -v```.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I modify the number of # to keep the consistent coding style.

Comment on lines +173 to +176
# The following command is the original commend, and it will fail on pytest == 8.0.0 .
# pytest --rootdir=/tmp -v
# Here is the issue and temporary solution: https://github.com/pytest-dev/pytest/issues/11781
# The alternative command to solve the issue is ```pytest --rootdir=. -v``` .
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I modify the number of # to keep the consistent coding style.

@yungyuc yungyuc merged commit 9ae2db3 into solvcon:master Feb 2, 2024
11 checks passed
@yungyuc
Copy link
Member

yungyuc commented Feb 2, 2024

Thanks @ThreeMonth03 for working around the pytest issue (pytest-dev/pytest#11781)

@yungyuc yungyuc added the test testing and continuous integration label Feb 2, 2024
j8xixo12 pushed a commit to j8xixo12/modmesh that referenced this pull request Mar 1, 2024
Work around the pytest issue pytest-dev/pytest#11781 by replacing `pytest rootdir=./tmp -v` with `pytest rootdir=. -v`

Co-authored-by: ThreeMonth03 <sankagetsu.cs08@nycu.edu.tw>
@ThreeMonth03 ThreeMonth03 deleted the debug branch March 6, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test testing and continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants