Skip to content

Add references#103

Merged
Jonas-Kirchhoff merged 40 commits intomainfrom
add_references
Oct 14, 2025
Merged

Add references#103
Jonas-Kirchhoff merged 40 commits intomainfrom
add_references

Conversation

@Jonas-Kirchhoff
Copy link

Multiple small changes addressing open issues of TSF.

  • add SHA-checker validating that single_include/nlohmann/json.hpp (the only file of the library that is needed) has the expected SHA
  • clean up the tables below the graphs in the trustable report
  • add comments to known open issues labeled as bug
  • implement test-result checker for referenced tests

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Oct 10, 2025
@github-actions github-actions bot added the L label Oct 13, 2025
@coveralls
Copy link

coveralls commented Oct 13, 2025

Coverage Status

coverage: 99.186%. remained the same
when pulling 23660ee on add_references
into 74f3891 on main.

@github-actions github-actions bot removed the L label Oct 14, 2025
@github-actions github-actions bot added the L label Oct 14, 2025
@Jonas-Kirchhoff Jonas-Kirchhoff marked this pull request as ready for review October 14, 2025 06:30
@@ -234,7 +234,7 @@ The total score is the mean of the individual scores.

## check_test_results
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it intentional that e.g., PJD-04 is still using check_artifact_exists and not check_test_results?

Copy link
Author

Choose a reason for hiding this comment

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

That slipped right through my net, thanks a lot!

---

The nlohmann/json library project CI executes on each pull request (opened, reopened, synchronized) the integration test suite, and failures in these runs are investigated by contributors.
The CI pipeline of the main branch executes on each pull request (opened, reopened, synchronized) the integration test suite, and failures in these runs are investigated by contributors.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it really be main branch? not e.g., default branch?

Copy link
Author

Choose a reason for hiding this comment

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

Not necessarily. This is one of the points that needs to be adapted when we bring it into S-Core, where the default is currently the develop branch of nlohmann/json.
You are, however, absolutely right that it would ideally be the default branch, in particular in view of the scheduled workflows.

score = 0.0
exceptions = []
try:
my_sha = hashlib.sha256(open(file,"rb").read()).hexdigest
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume it should be .hexdigest() and not .hexdigest ?

Copy link
Author

Choose a reason for hiding this comment

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

Indeed.

relevant_open_issues = [all_open_issues[i].get("number",None)
for i in range(0,len(all_closed_issues))
if len(all_closed_issues[i].get("labels",[]))!=0
and (all_closed_issues[i].get("labels"))[0].get("name") == "kind: bug"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't it be "all_open_issues" in the for loop here and not all_closed_issues?

Copy link
Author

Choose a reason for hiding this comment

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

Yes

id = int(entries[0])
except ValueError:
continue
if len(entries)>1 and entries[1].strip().capitalize=="NO":
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this work? I thought methods had to be called with parenthesis like ".capitalize()", otherwise you are referencing the method object itself rather than calling it.

Copy link
Author

Choose a reason for hiding this comment

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

No, for two reasons: First, the brackets are missing, and second, the method is wrong.

cols = line.split("|")
if cols[0].strip() == str(id) and len(cols)>2:
# Does the issue apply to us?
if cols[1].strip().capitalize == "NO":
Copy link
Collaborator

Choose a reason for hiding this comment

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

capitalize()??
see above comments

@Jonas-Kirchhoff Jonas-Kirchhoff merged commit 8c92b80 into main Oct 14, 2025
83 checks passed
@Erikhu1 Erikhu1 deleted the add_references branch November 4, 2025 15:16
Erikhu1 pushed a commit that referenced this pull request Nov 14, 2025
* add sha-checker

* add expected sha of json.hpp
* add validator checking if actual and expected sha coincide

* remove TODO-note

* refer to announcement of Niels Lohmann containing SHA-value

* clarify "without throwing an exception"

it does not mean that precision is guaranteed.

* add comments to known open issues

* collect comments on known open issues in TSF/docs/nlohmann_misbehaviours_comments.md
* print comments into collection of known issues.

* describe issue 4901

* refer to nlohmann docu for from_ubjson

* adapt file to reworked data storage

* adapt statement to better fitting one

* first draft of update process

* indicate that no known issue applies to Eclipse S-CORE

* add further comments

* add recommended procedure

* more tweaks to the process

* amend recommended procedure

* minor tweaks

* add branch protection rules

* adapt check_test_results

* replace "if not XXX" by more appropriate "if XXX is None"

* remove evidence of non-leaf item NJF-06.4

* remove evidence from non-leaf item NJF-06.5

* implement test-checker

* clean up graph's tabular presentation

* clean table from not rendered style

* fix path

* adapt README

* remove update process for further investigation

* adapt reference

* comment issue 4946

* add AOU that bugs are regularly checked on impact

* adapt table of known open misbehaviours

* add misbehaviour validator

* add .dotstop.dot containing AOU-28

* reorder workflow

* auto-update JLS-14

* validate PJD-04

* adapt to review_round_2

* fix brackets and method-name

* fix wrong list

* fix wrong method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI documentation Improvements or additions to documentation L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants