Reading annotation's parameters

[Macarse]

I would like to get an extension to #1

Apart from having the annotation, I would like to be able to read the parameters.
TestMethod could have an annotations: List<Annotation> instead of annotationNames: List<String> where Annotation contains the name + parameters dictionary.

Use case:
We are using Test Rail to track our regressions.
We would like to start moving as much as manual testing to automation as we can.

The idea is annotating test methods with an id to be able to update the test rail regression run with a pass/failure.

Example:

Android app will contain a test like this:

public class BasicJUnit4WithAnnotations {

  @Test
  @TestRail(id=1337)
  public void basicJUnit4WithAnnotations() {
    // Test something
  }
}

We would use a script which would create a dictionary of test class -> testRail id.
Example:
"com.linkedin.parser.test.junit3.java.JUnit3WithAnnotations#testJUnit3WithAnnotations":1337

We would run all the tests annotated with TestRail (we are using Flank: https://github.com/TestArmada/flank)

We would update testRail using the map created in 2) and the result from the automation run.

PS: I know I can add the id to the name of the method but I would prefer using the annotation since it's cleaner.

[Macarse]

ping?

[drewhannay]

To me, this feels out of scope for dex-test-parser, since the information gathered isn't actually needed to determine which tests to run. We've had requests for this before (see #8 ) but I don't think it's something that dex-test-parser should be handling.

One alternative solution for you, if you're always going to be building the test code before running tests, you could use an annotation processor to collect this information and write it to a file.

[Macarse]

re: out of scope for dex-test-parser:
I was reading your code and I notice you are already parsing the values into EncodedValue. It just needs a better structure to be usable from the API.

I have been playing with the code to implement this change myself. So far I didn't have issues parsing ints/boolean from the EncodedValue objects but I am having issues with strings. I don't fully understand how to use ParseUtils to get the string value from the dex.
How can I get the string value of the EncodedValue's value?

re: annotation processor:
I could do that, but I would prefer to add this small functionality to dex-test-parser.

Other use cases I can think of include:

• having the device where the test should be run
• Special configuration like portrait/landscape

[drewhannay]

Yeah, there's a fair amount of stuff that needs to be parsed but ultimately isn't used, just because of how the code is written. I understand that it probably would be easy to add this, but I don't want this project to slowly grow into a full blown dex decompiler :)

I already mentioned the annotation processor idea, but another one that we actually use for our tests is just to have the test itself write out any extra result info either to logcat or to a file on disk. Then after each test run, we pull that info off the device and process it. That approach also could cover your Test Rail use case.

For portrait vs landscape, you can check out another open source project from Linkedin: Test Butler, which supports letting you set / change the orientation of the device directly from tests.

[Macarse]

@drewhannay if you are reading information after the test is run, I suppose that's dynamic information, not static as the problem I am trying to solve.

I don't agree that parsing the annotation value is turning dex-test-parser into a dex decompiler, but you are the maintainer, so you will make the final call on what gets added and what does not.

Do you mind answering how can I get the string value of the EncodedValue's value?

[drewhannay]

I haven't tried this, but I'm guessing your issue is that ParseUtils::parseStringBytes expects to read the string out of a ByteBuffer, but the EncodedValue has already turned that part of the buffer into a byte array. I would think you could just construct a new String out of the byte array in EncodedValue.value and that would work.

[adecker89]

I have another use case of this feature which I think would be within the scope of determining which tests to run.

I'm currently making use of the standard Junit Category annotation to separate out my tests.
https://junit.org/junit4/javadoc/4.12/org/junit/experimental/categories/Category.html

With this setup, the annotation name alone is not enough for me to determine whether or not to run the test. While I could create new annotations as a workaround, the standardized Category annotation is appealing because it can work for both my jvm and android tests.

[Macarse]

@drewhannay I missed your msg. @adecker89 thanks for the reminder :)

@drewhannay I tried to do what you said, but I couldn't make it work.
My example is the following:

public class BasicJUnit4WithAnnotations {

  @Test
  @TestRail(id="1")
  public void basicJUnit4WithAnnotations() {
    assertTrue(true);
  }
}

I added some logs around toDescriptorList() and what I see is:

EncodedValue com.linkedin.dex.spec.EncodedValue@7699a589
.value [B@58372a00
ByteArray size 2
ByteArray forEach: 64
ByteArray forEach: 2

if I replace the id "1" for a longer string like "hello world!"
my log shows:

EncodedValue com.linkedin.dex.spec.EncodedValue@7699a589
.value [B@58372a00
ByteArray size 2
ByteArray for each: 62
ByteArray for each: 53

It seems like the byte array contains an address instead of the full value...

[drewhannay]

@Macarse Check this part of the dex format spec for the exact details of how these bytes are encoded. It looks like it varies based on the type of data in the EncodedValue. Sorry, my last reply was from my phone and from memory rather than checking the spec.

@adecker89 Okay, I agree, that's probably a reasonable use case. And since it also helps solve @Macarse 's use case, I'd be okay with adding this feature. If @Macarse gets it working and wants to send a pull request, that'd be great (otherwise I can work on this at some point in the future, not sure when I'd get to it).

[Macarse]

This is what I know so far:

Based on the docs, if the encoded value is VALUE_STRING the value should be an unsigned (zero-extended) four-byte integer value, interpreted as an index into the string_ids section and representing a string value.

As you can see from my logs, the value ByteArray contains only two bytes, not 4.

Playing a bit with the code, I did the following:

dexFile.stringIds.forEachIndexed { index, value ->
    dexFile.byteBuffer.position(value.stringDataOff)
    val myString = ParseUtils.parseStringBytes(dexFile.byteBuffer)
    if (myString.contains("hello world!")) {
        System.out.println("myString: " + myString + " stringId " + it + " with index: " + index)
    }
}

as a result I got:

myString: 
          hello world! stringId AnnotationOffItem(annotationOff=3121048) with index: 13630

I played with my ByteArray to match that 13630 and I got the correct logic by doing:

val barray = encodedValue.value as ByteArray
val buffer = ByteBuffer.allocate(4)
buffer.order(ByteOrder.LITTLE_ENDIAN)
buffer.put(barray[0])
buffer.put(barray[1])
buffer.put(0)
buffer.put(0)
buffer.flip()

val result = buffer.int

The name of the field is also parsable by doing:

val element = x.encodedAnnotation.elements.get(0)
System.out.println("AnnotationElement: " + element)
dexFile.byteBuffer.position(dexFile.stringIds[element.nameIdx].stringDataOff)
val myString = ParseUtils.parseStringBytes(dexFile.byteBuffer)
System.out.println("value " + myString) // outputs: value hello

@drewhannay how would you like to change the API?

I was thinking of modifying:
fun DexFile.getMethodAnnotationDescriptors(methodId: MethodIdItem, annotationsDirectory: AnnotationsDirectoryItem?): List<String>

and returning a Map<String, Map<String, Object>>

so something like:

@Test
@TestRail(hello="hello world!", other=1234)
public void basicJUnit4WithAnnotations() {
  assertTrue(true);
}

would end up being:

{"Test":{},
 "TestRail": {"hello":"hello world!", "other":1234}
}

[drewhannay]

Sorry for the delay again. @kkoser from my team has been working on this and should have a PR soon.

[kkoser]

Hey super sorry! I had to step away from this for a bit, but I'm back now and I will have this up by Friday. Sorry again!

[kkoser]

See #11

[bootstraponline]

Is this issue safe to close? It looks like the PR was merged.

[drewhannay]

Yep, good call.

[Macarse]

Thanks!