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

Testharnish #28

Merged
merged 14 commits into from
Jul 29, 2024
Merged

Testharnish #28

merged 14 commits into from
Jul 29, 2024

Conversation

0sak
Copy link
Contributor

@0sak 0sak commented Jul 5, 2024

#25 Testkonzept und -umgebung für Builder und Runtime

Additions:

  • CBuilder JUNIT Tests
  • CBuilder JUNIT System-Tests
  • C-Runtime Catch2 System-Tests
  • Gtihub Action test_suite.yml

@0sak 0sak assigned cagix and 0sak Jul 5, 2024
@0sak 0sak requested a review from cagix as a code owner July 5, 2024 10:29
Copy link
Member

@cagix cagix left a comment

Choose a reason for hiding this comment

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

@0sak Danke für den PR! Das ist mächtig viel Arbeit! 🙏

Die C++-Tests kriege ich irgendwie nicht an den Start - kannst Du da bitte nochmal schauen, ob man es irgendwie auch mit dem normalen g++/clang hinbekommen kann? Ich meine mich zu erinnern, dass Du da mal was gesagt hattest, aber ich krieg es nicht mehr zusammen.

Die Java-Tests laufen durch (./gradlew test) :)

Allerdings wirft ./gradlew spotlessJavaCheck ziemlich viele Fehler. Ich bin nicht sicher, was das alles ist, aber Dein Konfigurationsvorschlag ist auch recht umfangreich. Brauchen wir das wirklich so oder würde ein einfaches

spotless {
    java {
        googleJavaFormat().aosp().reflowLongStrings()
        target '**/*.java'
    }
}

in der build.gradle nicht auch reichen?

c-runtime/test/Systemtests/makefile Outdated Show resolved Hide resolved
@cagix
Copy link
Member

cagix commented Jul 5, 2024

@0sak Noch eine Idee: Könntest Du den Github-Workflow auskoppeln und als eigenen PR rüberschieben? Dann könnte ich den nämlich schon mal ins Repo einbauen, und dann würden wir hier die CI bereits nutzen können ...

@0sak
Copy link
Contributor Author

0sak commented Jul 8, 2024

@cagix PR-Workflow only

@0sak
Copy link
Contributor Author

0sak commented Jul 8, 2024

Spotless sollte mit dem von oben jetzt keine Fehler mehr werfen? Schau bitte mal, ob das bei dir auch so ist.

@cagix
Copy link
Member

cagix commented Jul 9, 2024

@0sak Ich habe den GH-Workflow im master korrigiert. Der Workflow selbst läuft jetzt - die Tests irgendwie nicht. Hmmm.

Bitte mal ein Rebase von Deinem Branch machen und dabei insbesondere bei .github/workflows/test_suite.yml die Änderungen aus dem master übernehmen und bei c-runtime/Makefile musst Du Deine Änderungen übernehmen. Also aufpassen :) ... Danach ein force-push in Dein Repo, und dann sollte hier der aktuelle Stand auftauchen und in der CI testbar sein.

Btw: Das Spotless habe ich aus dem Workflow für das Testen rausgenommen - der würde ja nur den normalen Java-Quellcode formatieren. Hier geht es aber um die Tests ... Das Spotless-Geraffel müsste man in einen zweiten Workflow einbauen, der bei Änderungen auf dem normalen Code losläuft.

@0sak
Copy link
Contributor Author

0sak commented Jul 16, 2024

@cagix der Java-Teil scheint zu laufen. Während der C-Teil mit dem image macOS-12 durchläuft, aber mit macOS-latest vor die Wand fährt. (Beispiel macOS-12) Ich weiß leider nicht, woran das liegt...

Ubuntu-latest kennt kein g++-13 alias und wirft nur mit g++ Fehler Beispiel ubuntu-latest. Auch mit dem image Beispiel ubuntu-24.04

@cagix
Copy link
Member

cagix commented Jul 17, 2024

@cagix der Java-Teil scheint zu laufen. Während der C-Teil mit dem image macOS-12 durchläuft, aber mit macOS-latest vor die Wand fährt. (Beispiel macOS-12) Ich weiß leider nicht, woran das liegt...

Ubuntu-latest kennt kein g++-13 alias und wirft nur mit g++ Fehler Beispiel ubuntu-latest. Auch mit dem image Beispiel ubuntu-24.04

@0sak ja, der java-teil läuft durch. im c-teil ist es dieses seltsame g++13-konstrukt.

ich muss noch verstehen, was da genau passiert. afaik gibt es keinen c++-13-standard, zumindest kann ich sowas nicht per option -std= angeben/auswählen. warum jetzt aber ein alias für das g++-binary hier auf einmal ein anderes verhalten erzeugt, kann ich grad noch nicht sagen - wenn das wirklich was in bezug auf den standard wäre, dann müsste das ja auch über die option klappen. ich hege die vermutung, dass da im catch2 irgendwas schräg ist, aber das muss ich nochmal nachlesen.

edit: mir bereitet es auch ein wenig sorge, dass die tests nur mit einem ziemlich alten standard laufen - das spricht dafür, dass irgendein deprecated verhalten genutzt wurde. ob nun bei dir oder (vermutlich eher) in catch2, ich muss da irgendwann mal beigehen.

was ich abgesehen davon noch unschön finde, ist dass der "g++13" im makefile fest und hart reinkodiert ist. könntest du bitte eine variable CC am anfang anlegen und die mit "g++13" belegen und dann bei den aufrufen weiter unten im makefile diese variable nutzen? dann hat man nur noch eine stelle, die man später ändern muss :)

edit: was passiert eigentlich, wenn man beim ausführen im github-runner sich so einen alias anlegt wie du es unter macos direkt gemacht hattest?

@0sak
Copy link
Contributor Author

0sak commented Jul 20, 2024

@cagix das hier wird eher wieder zu einem WIP als ein fertiger PR... Vielleicht doch nochmal schließen?

Ich hab mal geschaut, ob jemand ähnliche Probleme hatte und bin darauf gestoßen. catchorg/Catch2#2438 In einem separaten branch mal auf catch2 v3.x aufgestockt. Ein paar Tests, die error throws erwarten und ein komisches Verhalten mit dem ! operator mussten gelöscht werden, aber die Tests laufen soweit unter Ubuntu-latest.

Die Tage würde ich nochmal Clang unter macOS testen. Das mit dem alias habe ich noch nicht probiert.

@cagix
Copy link
Member

cagix commented Jul 21, 2024

oh, das sind gute nachrichten!

lass den pr gern offen, aber du kannst ihn ja auf "draft" stellen. (musst du aber auch nicht unbedingt - ich bin leider noch nicht dazu gekommen, mich näher damit auseinander zu setzen, die klausuren haben mich voll im griff.)

c-runtime/Makefile Outdated Show resolved Hide resolved
0sak and others added 3 commits July 26, 2024 16:10
* macos-12 workflow

* test macos-latest-large

* macos-latest

* ubuntu-latest

* g++-13 -> g++

* macos-latest with g++ instead g++-13

* macos-latest only

* ubuntu-24.04 beta

* CC makefile variable

* macos-latest

* easier testing

* Upgrade Catch2 v2.x to 3.x

* ubuntu-latest, all tests

* c++17 standard

* c++14 standard

* ubuntu only

* macos-latest only

* ubuntu-latest

* g++ instead of alias

* test

* clang

* g++-13 test

* check os in makefile

* Add ubuntu-latest

* d

* d

* conditional CC

* delete catch files and add java tests
Copy link
Member

@cagix cagix left a comment

Choose a reason for hiding this comment

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

ein eintrag im .gitignore für die beiden catch-downloads wäre auch sinnvoll 😎

c-runtime/test/src/catch_amalgamated.cpp Outdated Show resolved Hide resolved
c-runtime/Makefile Show resolved Hide resolved
@cagix
Copy link
Member

cagix commented Jul 26, 2024

@0sak die tests laufen durch auf der ci 😀🥳

c-runtime/Makefile Outdated Show resolved Hide resolved
Copy link
Member

@cagix cagix left a comment

Choose a reason for hiding this comment

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

@0sak Danke Dir! Ich glaube, das passt jetzt 🙃🥂 Danke für Deinen super Beitrag 🙏🤘

@cagix cagix merged commit cd3eb3d into Compiler-CampusMinden:master Jul 29, 2024
4 checks passed
cagix pushed a commit that referenced this pull request Jul 29, 2024
Additions:
* CBuilder JUNIT Tests
* CBuilder JUNIT System-Tests
* C-Runtime Catch2 System-Tests
* Gtihub Action test_suite.yml
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.

2 participants