-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
fix(Diagram): fix persisted data due to db not being cleared before parsing #3310
fix(Diagram): fix persisted data due to db not being cleared before parsing #3310
Conversation
sequenceDiagram | ||
Alice->Bob:Hello Bob, how are you? | ||
Bob-->Alice: I am good thanks!`); | ||
expect(diagram1.db.getMessages()).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If preferred, we can change this to a normal assertion, but I found an inline snapshot to be the fastest for understanding what is going on.
This is how it looks likes when this.db.clear()
is not called:
describe('more than one sequence diagram', () => {
it('should not have duplicated messages', () => {
const diagram1 = new Diagram(`
sequenceDiagram
Alice->Bob:Hello Bob, how are you?
Bob-->Alice: I am good thanks!`);
expect(diagram1.db.getMessages()).toMatchInlineSnapshot(`
Array [
Object {
"from": "Alice",
"message": "Hello Bob, how are you?",
"to": "Bob",
"type": 5,
"wrap": false,
},
Object {
"from": "Bob",
"message": "I am good thanks!",
"to": "Alice",
"type": 6,
"wrap": false,
},
]
`);
const diagram2 = new Diagram(`
sequenceDiagram
Alice->Bob:Hello Bob, how are you?
Bob-->Alice: I am good thanks!`);
expect(diagram2.db.getMessages()).toMatchInlineSnapshot(`
Array [
Object {
"from": "Alice",
"message": "Hello Bob, how are you?",
"to": "Bob",
"type": 5,
"wrap": false,
},
Object {
"from": "Bob",
"message": "I am good thanks!",
"to": "Alice",
"type": 6,
"wrap": false,
},
Object {
"from": "Alice",
"message": "Hello Bob, how are you?",
"to": "Bob",
"type": 5,
"wrap": false,
},
Object {
"from": "Bob",
"message": "I am good thanks!",
"to": "Alice",
"type": 6,
"wrap": false,
},
]
`);
// Add John actor
const diagram3 = new Diagram(`
sequenceDiagram
Alice->John:Hello John, how are you?
John-->Alice: I am good thanks!`);
expect(diagram3.db.getMessages()).toMatchInlineSnapshot(`
Array [
Object {
"from": "Alice",
"message": "Hello Bob, how are you?",
"to": "Bob",
"type": 5,
"wrap": false,
},
Object {
"from": "Bob",
"message": "I am good thanks!",
"to": "Alice",
"type": 6,
"wrap": false,
},
Object {
"from": "Alice",
"message": "Hello Bob, how are you?",
"to": "Bob",
"type": 5,
"wrap": false,
},
Object {
"from": "Bob",
"message": "I am good thanks!",
"to": "Alice",
"type": 6,
"wrap": false,
},
Object {
"from": "Alice",
"message": "Hello John, how are you?",
"to": "John",
"type": 5,
"wrap": false,
},
Object {
"from": "John",
"message": "I am good thanks!",
"to": "Alice",
"type": 6,
"wrap": false,
},
]
`);
});
});
Notice that we expect 2 messages per each diagram, but in the last diagram we have 6 messages.
For the E2E test, I did notice there is a test that tests the use case I was trying to tackle. and |
Good catch! Much appreciated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
📑 Summary
Data was being persisted to subsequent newer mermaid Diagrams called via
new
.Resolves #3305
📏 Design Decisions
I noticed that the
this.db
was not cleared after each call tonew Diagram(...)
. Since thedb
for each diagram type is a singleton, we need to to call clear.Otherwise, subsequent calls of the same type will persist the data to the next diagram created.
📋 Tasks
Make sure you
develop
branch