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

e2etests-cli: Add PNG test #963

Merged
merged 9 commits into from
Mar 3, 2023
Merged

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Mar 3, 2023

Closes #903
Closes #918

@nhooyr nhooyr requested review from alixander and a team March 3, 2023 03:45
@nhooyr nhooyr force-pushed the cli-tests-740f-b475 branch from 1fef587 to 9227021 Compare March 3, 2023 03:50
@nhooyr nhooyr marked this pull request as ready for review March 3, 2023 03:50
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

can u add a test with flag just to demo too? like different pad or something.
maybe a multi-board one too? to show how that'd work

i wonder why this png looks blurry. are you on latest d2 or release d2?

@nhooyr
Copy link
Contributor Author

nhooyr commented Mar 3, 2023

i wonder why this png looks blurry. are you on latest d2 or release d2?

This uses my xmain PR to run d2 in memory as though it's running externally. So it's the same commit which was just rebased on master.

@nhooyr
Copy link
Contributor Author

nhooyr commented Mar 3, 2023

Oh haha looks like I didn't run with TA=1 after rebasing.

@alixander
Copy link
Collaborator

actually the blurry png was last release, so it should've sorted itself. weiiiird. let me try pulling and running and seeing if i get a different PNG than u after u run with TA=1

@nhooyr
Copy link
Contributor Author

nhooyr commented Mar 3, 2023

PNGs aren't blurry for me after I ran with TA=1.

@nhooyr nhooyr requested a review from alixander March 3, 2023 04:03
@nhooyr nhooyr enabled auto-merge March 3, 2023 04:06
@nhooyr
Copy link
Contributor Author

nhooyr commented Mar 3, 2023

Fixed ✅

@nhooyr nhooyr mentioned this pull request Mar 3, 2023
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

Screen Shot 2023-03-02 at 8 28 26 PM

looks like they do fail in CI =(

@alixander
Copy link
Collaborator

maybe u need to install dependencies differently like this guy: #744 (comment)

@nhooyr nhooyr force-pushed the cli-tests-740f-b475 branch 3 times, most recently from 202b6cf to ff0aa99 Compare March 3, 2023 04:34
@nhooyr nhooyr requested a review from alixander March 3, 2023 04:35
@nhooyr nhooyr force-pushed the cli-tests-740f-b475 branch from ff0aa99 to 8bf6c97 Compare March 3, 2023 04:35
@nhooyr nhooyr requested a review from alixander March 3, 2023 04:41
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

Screen Shot 2023-03-02 at 8 41 39 PM

sadly the sketch is still a problem. let's skip/disable that test until the issue is found/fixed

@nhooyr
Copy link
Contributor Author

nhooyr commented Mar 3, 2023

Do you want me to disable d2renderer sketch tests too then? You can just push a commit after to make it work on arm64 and I'll remember to not touch.

@alixander
Copy link
Collaborator

Do you want me to disable d2renderer sketch tests too then? You can just push a commit after to make it work on arm64 and I'll remember to not touch.

no those don't compare diffs remember, they just output it so that the visual changes are shown in PRs

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

so good to have these, covering a huge blind spot for d2. i'm sure many bugs will be found as we populate these tests. 💯

@nhooyr nhooyr force-pushed the cli-tests-740f-b475 branch from f4f02da to 86de922 Compare March 3, 2023 04:51
nhooyr added 2 commits March 2, 2023 21:20
PNG renders seem to work intermittently on github actions so whatever.
If we're ignoring the diffs we might as well not even run these on CI.
@nhooyr nhooyr merged commit 09544af into terrastruct:master Mar 3, 2023
@nhooyr nhooyr deleted the cli-tests-740f-b475 branch March 3, 2023 05:34
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.

test PNG exports cli testing framework
2 participants