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

Break tests into different groups #323

Merged
merged 6 commits into from
Jul 2, 2021
Merged

Break tests into different groups #323

merged 6 commits into from
Jul 2, 2021

Conversation

devmotion
Copy link
Member

Fixes #315.

Probably it can/should be optimized what groups and in particular how many of them we use.

@devmotion
Copy link
Member Author

Maybe we could also drop tests on MacOS and Windows? There are no OS-specific features in KernelFunctions, so I don't think the additional tests provide any additional information. And if there are actually OS-specific bug reports at some we can still reevaluate.

@willtebbutt
Copy link
Member

Maybe we could also drop tests on MacOS and Windows?

I am in favour of this.

st--
st-- previously approved these changes Jul 1, 2021
Copy link
Member

@st-- st-- left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just a thought, to minimise the risk of someone at some point introducing a typo in "All", how about changing it to GROUP = get(ENV, "GROUP", "") and then simply testing if GROUP == "" || ...? The intent is the same ("" == "no specific group").

test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
st--
st-- previously approved these changes Jul 1, 2021
@devmotion
Copy link
Member Author

@st-- I noticed that the coveralls requires some explicit handling of the parallel runs, otherwise the coverage reports are completely messed up. I just updated the action according to https://github.com/coverallsapp/github-action#complete-parallel-job-example. Can you re-approve if you are happy with the PR?

Copy link
Member

@st-- st-- left a comment

Choose a reason for hiding this comment

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

I'm happy with it but there still seem to be some test issues...

@devmotion
Copy link
Member Author

The test error with the Julia nightly is caused by JuliaArrays/AxisArrays.jl#200 and not related to KernelFunctions.

@devmotion devmotion merged commit 10ad9ee into master Jul 2, 2021
@devmotion devmotion deleted the dw/groups branch July 2, 2021 21:42
@st--
Copy link
Member

st-- commented Jul 2, 2021

@devmotion why would coverage have decreased? does this mean some tests got missed out in the split ?

@devmotion
Copy link
Member Author

I don't think the drop is real, I can't find any coverage changes in the more detailed output of coveralls. I assume it is just related to the change from non-parallel to parallel tests and uploads.

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.

Parallelise tests in build
3 participants