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.

Sign up for GitHub

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

Jump to bottom

Test on current release and fix test failures #592

Merged
merged 5 commits into from Aug 19, 2020
Merged

Test on current release and fix test failures #592

merged 5 commits into from Aug 19, 2020

Conversation

andreasnoack
Copy link
Member

We didn't test on 1.5 because we'd hardcoded the minor release numbers. Hence, we hadn't noticed that tests didn't pass with the current Julia release version (1.5). There were some changes to the RNG in Julia 1.5 which broke the tests. Hence, I've added a test dependency on StableRNGs to ensure reproducible test results across Julia versions. For one of the tests, I also increased n. It should have little effect on the total test time.

@andreasnoack
Test on current release version
cae443a
@andreasnoack
Add StableRNGs as a test dependency to ensure same test results
af044d6 
across Julia versions.

Increase n when testing weighted sampleing with replacement to avoid
test failure on Julia >= 1.5
@andreasnoack
Use Travis for Windows testing for simplicity
9a78ccc
@andreasnoack
Copy link
Member Author

I've enabled Windows testing on Travis since it now seems to work well. @nalimilan could you please disable Appveyor (and review)?

@nalimilan
Copy link
Member

It would be nice to keep testing on 32-bit. I think you need to use matrix in travis.yml.

@andreasnoack
Copy link
Member Author

It would be nice to keep testing on 32-bit.

I agree.

I think you need to use matrix in travis.yml.

It looks like just specifying arch "globally" now works.

@nalimilan
Copy link
Member

OK. You need a custom code to test on 32-bit Windows, but I always forget whether it’s useful or not. @ararslan knows.

@andreasnoack
Copy link
Member Author

Isn't 32 bit Linux sufficient?

@ararslan
Copy link
Member

Yeah I think 32-bit Linux is sufficient. Support for 32-bit Windows was bumped down a tier for Julia itself anyway.

@ararslan
Copy link
Member

ararslan commented Aug 18, 2020 •
edited

As an aside, Travis must have fixed the thing such that unsupported os/arch combinations are now automatically skipped instead of requiring explicit matrix: exclude: stuff. That's nice, but it seems to be skipping 32-bit Windows, which is fine here but seems like a bug in general.

nalimilan
nalimilan approved these changes Aug 19, 2020
View reviewed changes
Copy link
Member

@nalimilan nalimilan left a comment •
edited

Choose a reason for hiding this comment

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

OK, thanks. I’ve disabled AppVeyor, can you also drop the badge from README.md ?

EDIT: I don’t appear to have the permission to delete the AppVeyor project. Not a big deal but if somebody wants to try...

@andreasnoack andreasnoack merged commit 3e60f4e into master Aug 19, 2020
@andreasnoack andreasnoack deleted the an/ci branch August 19, 2020 11:48
@kleinschmidt kleinschmidt mentioned this pull request Aug 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nalimilan nalimilan nalimilan approved these changes

@mschauer mschauer mschauer approved these changes

@ararslan ararslan ararslan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@andreasnoack @nalimilan @ararslan @mschauer