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

excluding doctests for mac/win64 platform ,Make them consistent with amd64 #5730

Merged
merged 1 commit into from
Mar 27, 2023

Conversation

jiangzhx
Copy link
Contributor

@jiangzhx jiangzhx commented Mar 25, 2023

Which issue does this PR close?

excluding doctests for mac/win64 platform ,Make them consistent with amd64.
There is already a standalone doctests on amd64

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@jiangzhx jiangzhx force-pushed the excluding_doctests_on_mac_win branch from f810930 to 93df23d Compare March 25, 2023 03:04
@jiangzhx
Copy link
Contributor Author

BTW: cargo test(win64) is much slower than amd64 and mac

image

shell: bash
run: |
export PATH=$PATH:$HOME/d/protoc/bin
cargo test
cargo test --lib --tests --bins --features avro,jit,scheduler,json,dictionary_expressions
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is a reasonable idea to run with the same features on mac and windows. Thank you @jiangzhx

@alamb
Copy link
Contributor

alamb commented Mar 25, 2023

Can you please explain a little about the rationale for this change (why you made it)?

@jiangzhx
Copy link
Contributor Author

Can you please explain a little about the rationale for this change (why you made it)?

I found that cargo test (win64) is much slower than amd64 and mac.
is it because something else is being done in the win64 environment than in the amd64 environment.

if we can make them consistent with amd64, which would make it easier to identify reason .

@alamb alamb added the development-process Related to development process of DataFusion label Mar 27, 2023
@alamb
Copy link
Contributor

alamb commented Mar 27, 2023

🤔 interestingly the CI tests did not trigger on this PR, which I noticed when trying to check the timings. I will fix that too and hopefully we can see the effect on this PR

@alamb
Copy link
Contributor

alamb commented Mar 27, 2023

Testing CI with #5759

@alamb
Copy link
Contributor

alamb commented Mar 27, 2023

The tests on #5759 good to me. Thank you @jiangzhx 🙏

Screenshot 2023-03-27 at 3 57 46 PM

@alamb alamb merged commit 9694f6a into apache:main Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development-process Related to development process of DataFusion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants