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

Event file comparision has new result: Additional event #2866

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kt86
Copy link
Contributor

@kt86 kt86 commented Oct 13, 2023

The current MISSING_EVENT result was used in both direction of comparison.
For me it was not intuitive, to always look in what direction the comparision fails (which is the file with the missing event).

Therefore, I propose, a new result depending on the comparison direction running compare(file1, file2):

  • If the event is in file1, but missing in file2, the result stays MISSING_EVENT.
  • If it is the other way round (event not in file 1, but in file 2), the result will be ADDITIONAL_EVENT.

This follows the logic from many test infrastructures, that file1 is the "expected" result, while file2 is the "actual" data.


As a follow-up, I would like to suggest and discuss the following (see #2867 ):
At least for me, it would be useful to have the option that the comparison will not stop, once both files are different. Instead, the comparison should be continued until the end and at least print out some numbers, like:

  • no. of additional events: x
  • no. of missing events: y
  • ....
    If one could also (optionally) print this events out, that may also help further for debugging. At least if we have integration tests, comparing event-files.

IMO: This can be done by introduce an enum for the comparison setting and behave based on it, e.g. either abort, countDifferences, writeOutDifferences.

…lt will now be "additional event". This makes it a bit easier to see in which direction the diff is (missing or additional)
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.

None yet

2 participants