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

ARROW-15347: [Doc][Guide] Update testing section in new contributors guide #12375

Closed
wants to merge 7 commits into from

Conversation

AlenkaF
Copy link
Member

@AlenkaF AlenkaF commented Feb 9, 2022

This PR adds and restructures the content of the testing section of the New Contributor's Guide.

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

The changes in the R part are mostly changes in line wrapping?

docs/source/developers/guide/step_by_step/building.rst Outdated Show resolved Hide resolved
docs/source/developers/guide/step_by_step/testing.rst Outdated Show resolved Hide resolved

Good to read:
`Invoking pytest versus python -m pytest <https://docs.pytest.org/en/6.2.x/pythonpath.html#pytest-vs-python-m-pytest>`_
(bottom of the page).
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if we should give here a takeaway for our contributors? Or is there not a simple one? (I personally always use pytest .., and not python -m pytest out of habit, and didn't run into problems with my current setup)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I should change python -m pytest to pytest..., also to match https://arrow.apache.org/docs/developers/python.html#unit-testing

As for the takeaway, maybe instead of linking I could add this as a note:
"You can also run the tests with python -m pytest [...]which is almost equivalent to using pytest [...] directly, except that calling via python will also add the current directory to sys.path and can in some cases help if pytest [...] doesn't seem to work."

What do you think?

docs/source/developers/guide/step_by_step/testing.rst Outdated Show resolved Hide resolved
docs/source/developers/guide/step_by_step/testing.rst Outdated Show resolved Hide resolved
@AlenkaF
Copy link
Member Author

AlenkaF commented Feb 15, 2022

Yes, the R part is only styling (indentation, line wrapping) changes.

@jorisvandenbossche jorisvandenbossche changed the title ARROW-15347: [Doc][Guide] Testing ARROW-15347: [Doc][Guide] Update testing section in new contributors guide Mar 9, 2022
@ursabot
Copy link

ursabot commented Mar 9, 2022

Benchmark runs are scheduled for baseline = a5dc252 and contender = 0b10a17. 0b10a17 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.5% ⬆️0.13%] test-mac-arm
[Finished ⬇️1.79% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.34% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@AlenkaF AlenkaF deleted the ARROW-15347 branch March 9, 2022 10:57
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