Moved the model context from the TM class into the TM object

[raphaelahrens]

To make it possible to import other models into the current model without adding all elements the elements of the model are now stored in the TM object and not the class.

I addition I added some test models to check how they differ between each other and between the current master branch. But the result of the models have to be compared manual, because the UUID and the order often differ even though the resulting report and the diagrams are similar. Maybe it is possible to automate this process more.

Since I can only do manual checking it would be good if someone else checked if this PR is not changing the original behavior.

[izar]

@nineinchnick you're WAY better at this stuff than I am, do you mind taking a look?

[nineinchnick]

I'll try tomorrow, I just rebased #75, and that took like 2 hours so I'm spent.

[ghost]

CodeSee Review Map:

Review in an interactive map

View more CodeSee Maps

Legend

[raphaelahrens]

@nineinchnick I wanted to ask what the opinion is about this PR? Is it worth keeping it up to date with master?

[nineinchnick]

@nineinchnick I wanted to ask what the opinion is about this PR? Is it worth keeping it up to date with master?

Sorry, it's rather big and I'm struggling to find time to review it as a whole, not just nitpicking.

In the description you mentioned something about UUIDs being different and having to do manual tests. Can you try using random.seed(0)? We won't accept this without automated tests.

[raphaelahrens]

Yes I do set the seed to zero with import norandom. The issue is the order in which the elements are created. For example the test files test0.py and test1.py produce the same model. But the order of the element creation differs. To then check if the models are the same I have to manually look at the diff to see that the uuids are switched.

Maybe you can just ignore the commit with my test cases https://github.com/izar/pytm/pull/175/commits/623d1f59a33d2a97ed39f2c337e8b95423867c80 They might only be useful to me.

[raphaelahrens]

I added some unit tests which are similar to the original tests. While doing this I also found out that it is no longer necessary to set random.seed(0), see 3c0267a7.

But still as soon as the creation order differs the uuids will change, which is rather annoying when comparing two threat models that are the same except for the import. See diff seq.plantuml import_data/seq.plantuml for example.

[raphaelahrens]

Hi,

should I close the PR?

[izar]

Sorry, it looks like we are all busy with life I am looking for @nineinchnick 's review, but he seems to be unable at this time. Let's wait a bit more for his chiming in before we decide where this goes.

[izar]

Hi @raphaelahrens - I think this got superseded by #190 - please let me know if that's not the case..

[raphaelahrens]

Hi,

the problem is the commit is all over the place and keeping it in sync with upstream requires to much time.

It will be easier to start fresh.

[izar]

Got it.

On Fri, Apr 29, 2022, 01:23 Raphael Ahrens @.***> wrote:

Hi,

the problem is the commit is all over the place an keeping it in sync with upstream requires to much time.

It will be easier to start fresh.

— Reply to this email directly, view it on GitHub https://github.com/izar/pytm/pull/175#issuecomment-1112887164, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC2BAO45RXZZUOGPGGK6LDVHNW4DANCNFSM5ETFVLZA . You are receiving this because your review was requested.Message ID: @.***>