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

Perform tests in different operative systems #125

Merged
merged 9 commits into from
Nov 5, 2024

Conversation

PabRod
Copy link
Contributor

@PabRod PabRod commented Sep 27, 2024

No description provided.

@PabRod PabRod linked an issue Sep 27, 2024 that may be closed by this pull request
@PabRod
Copy link
Contributor Author

PabRod commented Sep 27, 2024

@mdingemanse, your hypotheses was 100% right. Looking into png documentation, I see they even have a section about Differences between OSes. I'll look into this.

@PabRod
Copy link
Contributor Author

PabRod commented Sep 27, 2024

The OS dependency of saved plots seems to be a serious issue. See for instance here. This is a big problem regarding snapshots.

@PabRod
Copy link
Contributor Author

PabRod commented Sep 27, 2024

Next step: take a look at ragg.

@mdingemanse
Copy link
Contributor

Just to note that while ragg may offer a solution for this micro issue, we risk getting rabbitholed quite far from the design goals of the package and the aims of the tests. For the functionality of talkr it is not important if there happens to be slight pixel level variation in images created in tests. And for tests, the main use case is a single coder iterating over some bit of code and using the tests on their own system (keeping OS constant) as a sanity check that they're not breaking anything.

I can live with the few times where we'd see a very subtle difference that is not a difference because we ran a test on different OSs. The main issue to solve when it comes to different Issues is the stray Rplots.pdf image being created on Windows, not *nix.

@PabRod PabRod self-assigned this Sep 30, 2024
@PabRod PabRod requested a review from mdingemanse September 30, 2024 09:37
@PabRod
Copy link
Contributor Author

PabRod commented Sep 30, 2024

Take a look, @mdingemanse. I hot-fixed it by running the snapshot tests conditionally, i.e.:, only in Linux.

Surprisingly, there doesn't seem to be a more elegant way of guaranteeing identical outputs across OSs.

@PabRod PabRod marked this pull request as ready for review October 11, 2024 08:27
Copy link
Contributor

@mdingemanse mdingemanse left a comment

Choose a reason for hiding this comment

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

okay for now, though I think the more elegant solution will be to break the plot saving code out of the function fully and expect users to find their own OS-specific ways of doing it

@PabRod PabRod merged commit 443fdcb into main Nov 5, 2024
5 checks passed
@PabRod PabRod deleted the 124-os-dependent-snapshots branch November 5, 2024 09:41
@PabRod PabRod mentioned this pull request Jan 9, 2025
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.

OS-dependent snapshots
2 participants