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

d2chaos: fixes chaos tests #394

Merged
merged 4 commits into from
Dec 6, 2022
Merged

Conversation

ejulio-ts
Copy link
Contributor

Fixed #376

Fixes the two issues in #376

Each commit should be a separate fix.

  • only add an edge between sequence diagrams if it is valid, otherwise skip it
  • removed the check for self edges in chaos tests as we support it now
  • sequence diagrams weren't expected in chaos tests, as images, but the ifs there allowed it
    • as I fixed the sequence diagrams issues, they are now allowed in chaos tests
    • images are still not allowed because they need an icon. I thought about adding some code to generate random URLs, but as we lived without it so far, I don't think this is critical, so skipped
  • fixed some other issues found with special chars in IDs with dagre
    • changed the test from dagre_id_with_new_line to dagre_id_with_special_chars and added the new examples to that test

@ejulio-ts ejulio-ts merged commit d541e44 into terrastruct:master Dec 6, 2022
@ejulio-ts ejulio-ts deleted the gh-376-d2chaos branch December 6, 2022 22:08
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

good stuff ✅

// fixes \\
id = strings.ReplaceAll(id, "\\", `\\`)
// replaces \n with \\n whenever \n is not preceded by \ (does not replace \\n)
re := regexp.MustCompile(`[^\\](\n)`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the paren around \n?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, for a moment I thought I had to replace the group and ended forgetting it there 😅

nhooyr added a commit to nhooyr/d2 that referenced this pull request Dec 8, 2022
@nhooyr nhooyr mentioned this pull request Dec 8, 2022
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.

d2chaos: respect sequence diagrams
3 participants