2010-05-11 16 views
80

Negli ultimi anni ho sempre pensato che in Java, Reflection è ampiamente utilizzato durante il test delle unità. Poiché alcune delle variabili/metodi che devono essere controllati sono privati, è in qualche modo necessario leggerne i valori. Ho sempre pensato che l'API Reflection sia utilizzata anche per questo scopo.È una cattiva pratica usare Reflection in Unit testing?

La scorsa settimana ho dovuto testare alcuni pacchetti e quindi scrivere alcuni test JUnit. Come sempre ho usato Reflection per accedere a campi e metodi privati. Ma il mio supervisore che ha controllato il codice non è stato molto contento e mi ha detto che l'API di Reflection non era pensata per l'uso di tale "hacking". Invece ha suggerito di modificare la visibilità nel codice di produzione.

È davvero una cattiva pratica usare Reflection? Non posso davvero credere che-

Edit: avrei detto che mi è stato richiesto che tutti i test sono in un pacchetto separato chiamato prova (in modo da utilizzare protetta visibilità ad esempio, non era una possibile soluzione troppo)

+0

Che tipo di accesso pratichi con la riflessione, l'impostazione, l'acquisizione o entrambi? – fish

+0

@fish: uso solo per verificare se alcuni valori specifici sono stati impostati – RoflcoptrException

+7

È normale pratica in Java inserire i test nello stesso pacchetto in modo da poter testare le classi "pacchetto privato" (accesso predefinito). –

risposta

58

IMHO Reflection dovrebbe essere solo l'ultima risorsa, riservata al caso speciale del codice legacy di test unitario o di un'API che non puoi modificare. Se stai testando il tuo codice, il fatto che devi usare Reflection significa che il tuo design non è testabile, quindi dovresti correggerlo invece di ricorrere a Reflection.

Se è necessario accedere ai membri privati ​​nei test di unità, di solito significa che la classe in questione ha un'interfaccia inadatta e/o tenta di fare troppo. Quindi, la sua interfaccia dovrebbe essere rivista, o qualche codice dovrebbe essere estratto in una classe separata, dove quei metodi problematici/accessori di campo possono essere resi pubblici.

Si noti che l'uso di Reflection genera in genere un codice che, oltre a essere più difficile da capire e mantenere, è anche più fragile. Esiste un'intera serie di errori che nel caso normale verranno rilevati dal compilatore, ma con Reflection vengono visualizzati solo come eccezioni di runtime.

Aggiornamento: come notato da @tackline, ciò riguarda solo l'uso di Reflection all'interno del proprio codice di test, non all'interno del framework di test. JUnit (e probabilmente tutti gli altri framework simili) usa la riflessione per identificare e chiamare i metodi di test: questo è un uso giustificato e localizzato della riflessione. Sarebbe difficile o impossibile fornire le stesse funzionalità e la stessa comodità senza usare Reflection. OTOH è completamente incapsulato all'interno dell'implementazione del framework, quindi non complica o compromette il nostro codice di test.

+9

Reflection è ovviamente utile all'interno del framework di test per chiamare test e interfacce di simulazione, in alternativa ai processori di annotazioni in fase di compilazione. Ma il cappello dovrebbe essere chiaramente indicato come un tipo separato di utilizzo. –

+1

@tackline, questo è quello che intendevo davvero, grazie per averlo fatto notare. Ho aggiunto un chiarimento alla mia risposta. –

+0

Non hai bisogno di molte qualifiche. La riflessione dovrebbe essere usata principalmente come ultima risorsa, periodo. Rende il ragionamento sul programma effettivamente impossibile. –

56

È davvero male modificare la visibilità di un'API di produzione solo per motivi di test. È probabile che tale visibilità sia impostata sul valore corrente per motivi validi e non debba essere modificata.

L'utilizzo della riflessione per il test dell'unità è in genere ottimale. Certo, dovresti design your classes for testability, in modo che sia necessaria una riflessione minore.

La primavera, ad esempio, ha ReflectionTestUtils. Ma il suo scopo è quello di mettere in giro delle dipendenze, dove la primavera avrebbe dovuto iniettarle.

Il tema è più profondo di "do & no", e considera cosa dovrebbe essere testato - se lo stato interno degli oggetti deve essere testato o meno; se dovremmo permetterci di mettere in discussione il design della classe sotto test; ecc.

+1

spiegheresti il ​​tuo downvote? – Bozho

+6

Ci sono due lati della modifica dell'API di produzione. Cambiarlo solo per i test è sbagliato, mentre aggiornare la tua API/design in modo che il tuo codice sia più testabile (e quindi non stai cambiando la tua visibilità solo per i test) è una buona pratica. – deterb

+1

se sei sicuro che ciò che stai facendo non ha effetti collaterali - sì. ;) – Bozho

7

Lo considererei una cattiva pratica, ma la semplice modifica della visibilità nel codice di produzione non è una buona soluzione, è necessario esaminare la causa.O hai un'API non testabile (cioè l'API non espone a sufficienza per un test per ottenere un handle), quindi stai cercando di testare lo stato privato, oppure i tuoi test sono troppo accoppiati con la tua implementazione, che li renderà di uso solo marginale quando si refactoring.

Senza saperne di più sul tuo caso, non posso davvero dire di più, ma in effetti è considerato una pratica inadeguata usare reflection. Personalmente preferirei fare del test una classe interna statica della classe sotto test piuttosto che ricorrere alla riflessione (se la parte non testabile dell'API non era sotto il mio controllo), ma alcuni posti avranno un problema più grande con il codice di test in lo stesso pacchetto del codice di produzione rispetto all'utilizzo della riflessione.

MODIFICA: in risposta alla modifica, che è una pratica almeno altrettanto scarsa dell'utilizzo di Reflection, probabilmente peggio. Il modo in cui viene gestito in genere consiste nell'utilizzare lo stesso pacchetto, ma mantenere i test in una struttura di directory separata. Se i test unitari non appartengono allo stesso pacchetto della classe sotto test, non so cosa faccia.

In ogni caso, è ancora possibile ottenere intorno a questo problema utilizzando protetta (purtroppo non il pacchetto-privato che è davvero l'ideale per questo) testando una sottoclasse in questo modo:

public class ClassUnderTest { 
     protect void methodExposedForTesting() {} 
} 

E dentro il tuo test di unità

class ClassUnderTestTestable extends ClassUnderTest { 
    @Override public void methodExposedForTesting() { super.methodExposedForTesting() } 
} 

E se si dispone di un costruttore protetto:

ClassUnderTest test = new ClassUnderTest(){}; 

Non consiglio necessariamente quanto sopra per situazioni normali, ma le restrizioni alle quali ti viene chiesto di lavorare e non le "migliori pratiche" già.

+0

+1 Per la modifica della visibilità del codice di produzione non è una buona soluzione –

3

Penso che il tuo codice debba essere testato in due modi. Dovresti testare i tuoi metodi pubblici tramite Unit Test e questo fungerà da nostro test della scatola nera. Poiché il tuo codice è suddiviso in funzioni gestibili (buon design), ti consigliamo di testare unitamente i singoli pezzi con la riflessione per assicurarti che funzionino indipendentemente dal processo, l'unico modo in cui posso pensare di farlo sarebbe con la riflessione sono privati.

Almeno, questo è il mio modo di pensare nel processo di test unitario.

28

Dal punto di vista di TDD - Test Driven Design - questa è una cattiva pratica. So che non hai taggato questo TDD, né lo chiedi espressamente, ma il TDD è una buona pratica e va contro il suo livello.

In TDD, utilizziamo i nostri test per definire l'interfaccia della classe: l'interfaccia pubblica- e quindi stiamo scrivendo test che interagiscono direttamente solo con l'interfaccia pubblica. Siamo interessati a tale interfaccia; i livelli di accesso appropriati sono una parte importante del design, una parte importante del buon codice. Se ti ritrovi a dover testare qualcosa di privato, di solito è, secondo la mia esperienza, un odore di design.

+3

concordato. Tratta l'oggetto da testare come una scatola nera - assicurati che le uscite corrispondano agli input. Se non lo sai, hai un problema con i componenti interni dell'implementazione. – AngerClown

+0

Ma per quanto riguarda testare lo stato dell'oggetto? –

+5

Lo stato interno dell'oggetto non ha importanza, purché il suo comportamento esterno sia corretto. –

3

Per aggiungere a ciò che altri hanno detto, considerare quanto segue:

//in your JUnit code... 
public void testSquare() 
{ 
    Class classToTest = SomeApplicationClass.class; 
    Method privateMethod = classToTest.getMethod("computeSquare", new Class[]{Integer.class}); 
    String result = (String)privateMethod.invoke(new Integer(3)); 
    assertEquals("9", result); 
} 

Qui, stiamo usando la riflessione per eseguire il metodo SomeApplicationClass.computeSquare privata(), passando un intero e restituisce un risultato stringa.Ciò si traduce in un test JUnit che compilare bene ma sicuro durante l'esecuzione se uno dei seguenti casi:

  • nome Method "computeSquare" viene rinominato
  • Il metodo prende in un diverso tipo di parametro (ad esempio cambiando intero a lungo)
  • Il numero di parametri cambio (es passando in un altro parametro)
  • Il tipo ritorno dei cambiamenti metodo (forse da stringa a intero)

Invece, se c'è non è un modo semplice per dimostrare che computeSquare ha fatto quello che dovrebbe fare attraverso l'API pubblica, quindi probabilmente la tua classe sta cercando di fare molto. Tirare questo metodo in una nuova classe che vi dà il seguente test:

public void testSquare() 
{ 
    String result = new NumberUtils().computeSquare(new Integer(3)); 
    assertEquals("9", result); 
} 

Now (soprattutto quando si utilizzano gli strumenti di refactoring disponibili nel moderno IDE), la modifica del nome del metodo non ha alcun effetto sulla vostra prova (come il tuo IDE avrà anche rifattorizzato il test di JUnit, mentre cambiando il tipo di parametro, il numero di parametri o il tipo di ritorno del metodo contrassegno un errore di compilazione nel tuo test di Junit, il che significa che non verificherai un test JUnit che compila ma fallisce in fase di esecuzione.

Il mio ultimo punto è che a volte, specialmente quando si lavora in un codice legacy, e occorre aggiungere nuove funzionalità, potrebbe non essere semplice farlo in una classe ben scritta e testabile separata. In questo caso la mia raccomandazione sarebbe quella di isolare le nuove modifiche del codice ai metodi di visibilità protetta che è possibile eseguire direttamente nel codice di test JUnit. Questo ti permette di iniziare a costruire una base di codice di test. Alla fine dovresti rifattorizzare la classe ed estrarre le tue funzionalità aggiuntive, ma nel frattempo, la visibilità protetta sul tuo nuovo codice può a volte essere la soluzione migliore in termini di testabilità senza importanti refactoring.

5

I secondo il pensiero di Bozho:

È pessima modificare la visibilità di un'API produzione solo per il gusto di test.

Ma se si prova a do the simplest thing that could possibly work, allora potrebbe essere preferibile scrivere boilerplate di Reflection API, almeno mentre il codice è ancora nuovo/modificato. Voglio dire, l'onere di cambiare manualmente le chiamate riflesse dal tuo test ogni volta che cambi il nome del metodo, o parametri, è IMHO troppo pesante e l'attenzione sbagliata.

essere caduto nella trappola di accessibilità relax solo per le prove e poi inavvertitamente l'accesso al metodo era-privato da altro codice di produzione ho pensato di dp4j.jar: si inietta il codice Reflection API (Lombok-style) in modo da non modificare il codice di produzione E non scrivere da soli l'API di Reflection; dp4j sostituisce l'accesso diretto al test dell'unità con l'API di riflessione equivalente in fase di compilazione. Ecco un example of dp4j with JUnit.