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 updater for expected results of integration tests #542

Open
benmss opened this issue Nov 3, 2023 · 6 comments
Open

Add updater for expected results of integration tests #542

benmss opened this issue Nov 3, 2023 · 6 comments
Assignees
Labels
tests Enhancement of tests

Comments

@benmss
Copy link
Member

benmss commented Nov 3, 2023

Currently any changes that create a legitimate difference in Macaron's results and the expected results used in the integration tests must be handled manually. An automatic method for updating the expected results files would save considerable time moving forwards.

@benmss benmss self-assigned this Nov 3, 2023
@benmss benmss added the tests Enhancement of tests label Nov 3, 2023
@behnazh-w
Copy link
Member

Yes please! Looking forward to the PR 👍

@nathanwn
Copy link
Member

nathanwn commented Nov 3, 2023

Can we utilize syrupy for this purpose? It already supports comparing complex JSON structures and overwriting existing results.

@behnazh-w
Copy link
Member

behnazh-w commented Nov 3, 2023

Can we utilize syrupy for this purpose? It already supports comparing complex JSON structures and overwriting existing results.

We have our own custom scripts to process the JSON outputs. We trim the non-deterministic parts and only check the parts that are crucial. So, I doubt Syrupy being a good solution.

All we need to do really is to update the JSON outputs by copying over the files if an update flag is provided.

@nathanwn
Copy link
Member

nathanwn commented Nov 3, 2023

If we have non-deterministic parts in the expected result files, then can't we just "eliminate those non-deterministic parts" from these expected result files? These files can store just the "deterministic subset" of the JSON result that Macaron produces, and not the whole result. This way we may be able to delegate both comparing and updating to surupy and remove the burden of maintaining the compare script.
Here by "eliminate the non-deterministic parts", I mean both (1) literally removing certain things that we don't need to check for and (2) forcing certain things to be deterministic when Macaron outputs them, such as the order of lists in the output. Point (2) also helps to minimize the diff of these expected output files when we commit them, making it easier to inspect and code review.

@behnazh-w
Copy link
Member

This way we may be able to delegate both comparing and updating to surupy and remove the burden of maintaining the compare script.

Removing certain values from the output and storing it as intermediate files for testing also needs maintenance and can be even more complex. I don't see the benefit here since we already have comparison scripts.

@nathanwn
Copy link
Member

nathanwn commented Nov 3, 2023

Yep. You're right that in that case, we need to cherry-pick things from the current output, and that part requires maintenance. Something I am wondering is whether the compare script or the cherry-picking will require more maintenance long-term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Enhancement of tests
Projects
None yet
Development

No branches or pull requests

3 participants