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

Add Corrfunc/tests.py to CI testing #260

Merged
merged 2 commits into from
Oct 8, 2021
Merged

Add Corrfunc/tests.py to CI testing #260

merged 2 commits into from
Oct 8, 2021

Conversation

lgarrison
Copy link
Collaborator

From #258, we're calling the linear binning tests from Corrfunc/test.py, but it isn't CI'd. This PR starts CI-ing it.

An alternative would be to move the linear binning tests to [theory|mocks]/python_bindings/call_correlation_functions.py or a new file in the same directory. But following the "principle of least surprise", I think it makes sense to just CI Corrfunc/test.py, as most users/devs will expect it to be tested.

This is a blocking issue for #258, as we aren't actually testing anything yet!

@lgarrison lgarrison added this to the v2.5.0 milestone Oct 7, 2021
@manodeep
Copy link
Owner

manodeep commented Oct 8, 2021

Hmm - does the CI only on master run rather than that on the branch? In which case, perhaps I will just have to merge this directly without the GH CI running

@lgarrison
Copy link
Collaborator Author

The CI did run on the first commit in this PR, and then the second was just a changelog update so I did [ci skip] out of habit 😬. But it passed all the checks in the first commit, so as long as my changelog update wasn't too reckless, it should be safe to merge!

@manodeep
Copy link
Owner

manodeep commented Oct 8, 2021

Ahh yes - I now see that the CI ran under the Actions tab (rather than the checks tab associated with the PR). Looks good to me :)

@manodeep manodeep merged commit f93c902 into master Oct 8, 2021
@manodeep manodeep deleted the ci-test.py branch October 8, 2021 02:36
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.

None yet

2 participants