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

Fix/match display off-by-one error and reenable clustering display #816

Merged
merged 14 commits into from
Dec 12, 2022

Conversation

cyfml
Copy link
Contributor

@cyfml cyfml commented Nov 23, 2022

I fixed the bug: The ends of displayed matches are sometimes off by one and re-enable cluster. It is based on that file path problem is resolved (last PR).
In addition, i added a test case to test cluster.

@sebinside
Copy link
Member

Just a reminder from the discussion: Please also fix that the starting line is off by one

@tsaglam tsaglam requested review from sebinside and tsaglam November 29, 2022 13:43
@tsaglam tsaglam added bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Nov 29, 2022
Copy link
Member

@tsaglam tsaglam left a comment

Choose a reason for hiding this comment

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

This PR contains new test resources but no new test case? They are probably from testing and can be removed, right?

@cyfml
Copy link
Contributor Author

cyfml commented Dec 6, 2022

This PR contains new test resources but no new test case? They are probably from testing and can be removed, right?

Yes. After today's classes, i will take a look.

Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

The code looks okay to me, the match display also seems to work correctly now.

However, we noticed a bug in the ReportGenerator (not your fault, this bug probably existed for a long time). Sometimes, the submission IDs in the array of the cluster's members are serialized as null which breaks everything. I will try to have a look at it next week. If would be good if you could also try to replicate this behavior. Currently, I don't know why this happens.

Example:

{"average_similarity":0.31,"strength":3.72396E-5,"members":["SubmissionA","SubmissionB",null,null,null,null]}

@sebinside
Copy link
Member

We found another bug in the report generation, it was fixed with #842. Please rebase your PR so that we can test it again, afterward, it can be merged.

@cyfml
Copy link
Contributor Author

cyfml commented Dec 12, 2022

@sebinside already done.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

87.5% 87.5% Coverage
0.0% 0.0% Duplication

@tsaglam tsaglam changed the title Fix/match display and reenable cluster Fix/match display off-by-one error and reenable clustering display Dec 12, 2022
@tsaglam tsaglam merged commit e84a5ba into jplag:develop Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants