Conversation
|
Ottimo lavoro @FaberNa ne parliamo su #spid-spring di developers italia se vi va |
|
In questa PR c'è sicuramente da portare il plugin di maven per abilitare i test in build, poi per java 11 è tutto da decidere |
| softly.assertThat(resultElement.getAttributes().getNamedItem("AssertionConsumerServiceIndex").getTextContent()).isEqualTo(String.valueOf(assertionConsumerServiceIndex)); | ||
| softly.assertThat(resultElement.getAttributes().getNamedItem("ProtocolBinding").getTextContent()).isEqualTo("urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"); | ||
| softly.assertThat(resultElement.getAttributes().getNamedItem("IsPassive")).isNull(); | ||
| softly.assertThat(resultElement.getAttributes().getNamedItem("IsPassive")).isNotNull(); |
There was a problem hiding this comment.
@FaberNa mi pareva di aver capito (anche da questa guida qui) che l'attributo IsPassive non dovesse essere presente.
@peppelinux tu sei sicuramente più informato.
There was a problem hiding this comment.
@marque88 è corretto
consiglio di controllare metadata e authnrequest con spid-sp-test per stare tranquilli su eventuali anomalie da risolvere
There was a problem hiding this comment.
- nell’elemento non deve essere presente l’attributo IsPassive (ad indicare false come valore di default) ... in verità andrebbe aggiunto che se presente deve essere false
There was a problem hiding this comment.
un giro con spid-sp-test ci darebbe contezza sulle criticità, esempio:
Questi sono i controlli che AgID fa in fase di collaudo
|
|
||
| @SneakyThrows | ||
| @BeforeAll | ||
| static void startup(){ |
There was a problem hiding this comment.
Incorrect indentation
| static void startup(){ | |
| static void startup(){ |
| "-----END CERTIFICATE-----"; | ||
| idpKeyManager = new IdpKeyManager(entity,certificateStr); | ||
| } | ||
|
|
| softly.assertThat(credential.getEntityId()).isEqualTo("test.idp.entity"); | ||
| }); | ||
| } | ||
| @Test |
There was a problem hiding this comment.
Missing an empty line
| @Test | |
| @Test |
| Credential credential = idpKeyManager.getCredential(null); | ||
| assertThat(credential).isNull(); | ||
| } | ||
| @Test |
There was a problem hiding this comment.
Missing an empty line
| @Test | |
| @Test |
| Certificate certificate = idpKeyManager.getCertificate(null); | ||
| assertThat(certificate).isNull(); | ||
| } | ||
| @Test |
There was a problem hiding this comment.
Missing an empty line
| @Test | |
| @Test |
| void shouldReturnCredentialWhenKeyisNotNullAndKeyEqualEntity() { | ||
| Credential credential = idpKeyManager.getCredential("test.idp.entity"); | ||
| SoftAssertions.assertSoftly(softly -> { | ||
| softly.assertThat(credential).isNotNull(); |
There was a problem hiding this comment.
Have you considered whether this assertion could be redundant? Since if credential will ever be null, the next assertion would fail the test anyway.
| @SneakyThrows | ||
| @BeforeAll | ||
| static void startup(){ | ||
| String entity ="test.idp.entity"; |
There was a problem hiding this comment.
Missing a space
| String entity ="test.idp.entity"; | |
| String entity = "test.idp.entity"; |
There was a problem hiding this comment.
This string is repeated a few times throughout the tests, consider extracting it to a constant.
|
@polarene scusa vedo che hai corretto l'indentazione ma dentro al progetto non vedo file di style per il codice, non sarebbe il caso di aggiungere qualcosa stile check-style ? @peppelinux |
|
@FaberNa sì assolutamente, bisognerebbe aggiungere almeno il supporto CheckStyle con uno stile condiviso (ad es. Google Style, così da tagliare la testa al toro) e magari un file |
|
Oltre ad editorconfig, potremmo introdurre anche il plugin maven di Spotless, che consente di auto-formattare il codice durante la build. Supporta vari stili configurabili, incluso quello Google. Viva la piena automazione! ;) |
4 participants
@peppelinux Ho aggiunto qualche test e fatto il passaggio a java 11. Inoltre ho corretto l'errore per cui i test non partivano, senza il plugin corretto con junit5 non venivano considerati ...