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

Delayed writing of results to build/.../fileStates.txt #35

Open
danwallach opened this issue Apr 10, 2019 · 6 comments
Open

Delayed writing of results to build/.../fileStates.txt #35

danwallach opened this issue Apr 10, 2019 · 6 comments

Comments

@danwallach
Copy link

I'm building an autograder for my class, and I'm planning to use your tool to ensure students' code is properly formatted. My problem is that I need to parse fileStates.txt so I can give a summary in the autograder (e.g., "8/9 files are formatted; run gradle googleJavaFormat to fix this"). So far as I can tell, here's your relevant code:

    private FileInfoStore setupFileStore(FileInfoStore store, FileToStateMapper mapper) {
        project.gradle.buildFinished {
            try {
                store.update(mapper)
            } catch (IOException e) {
                project.getLogger().error("Failed to write formatting states to disk", e)
            } finally {
                store.close()
            }
        }
        return store
    }

I think my problem is that you're delaying the write until the build finishes, and I need this information as part of the same build. I'm not an expert on the guts of Gradle, but I think you want to do this after the project is finished, or just right away, rather than waiting until the entire build has finished.

@sherter
Copy link
Owner

sherter commented Apr 18, 2019

Hi, sorry for not getting back to you earlier.

First of all, the fileStates.txt file is by no means an API. It is used as a cache to speed up consecutive runs of format/verify tasks. The file's format can change at any time (for example, to something binary, like a SQLite or H2 database). I started out with a plain text file mainly for development convenience/ease of debugging.

I'm not sure I understand what you are trying to do. If you really want to grade some student's project, you can't use the result of running verifyGoogleJavaFormat inside that project, can you? I mean, what if he simply removed all the inputs from the task (exclude '*')? I think you should use google-java-format itself (not this plugin) and iterate over all the *.java files in the project directory. Or maybe I don't get the problem correctly...

As for your suggestion to persist the cache earlier in the build lifecycle, I'm not aware of any hook that allows me to do stuff after all tasks of a project were executed. It might be possible to somehow analyze the TaskExecutionGraph and sneak in some action to execute after the last format/verify task of a project, but I'd rather not introduce a hack in order to support another hack.

@danwallach
Copy link
Author

What I'm trying to do: Write an autograder which looks at all the ways a student could make mistakes and assign points to them. You can play with it if you want:

https://github.com/RiceComp215-Staff/RiceChecks

The top-level idea is that students do their work with GitHub. On every push, Travis-CI runs ./gradlew autograde, which then builds everything, runs all the tests, assigns points, and prints out the results. Human graders come along afterward to to make sure there were no shenanigans (e.g., hacking the build.gradle file).

I want to run everything, no matter what, and then collect up all the errors, rather than Gradle's usual policy of stopping the first time it finds a problem. As such, for code formatting, I've rigged up a task to run your plugin like so:

import com.github.sherter.googlejavaformatgradleplugin.VerifyGoogleJavaFormat
task autograderVerifyGoogleJavaFormat(type: VerifyGoogleJavaFormat) {
    source 'src/main'
    source 'src/test'
    include '**/*.java'
    ignoreFailures true
}

The autograder then looks for your fileStates.txt file, and similar artifacts left behind by CheckStyle, JaCoCo, and the JUnit test runner, to arrive at a student's grade. The only task that doesn't leave behind any evidence in the filesystem turns out to be running the Java compiler itself. For that, I have Gradle redirect stdout to a file.

Certainly, I could rig up my autograder to directly run GoogleJavaFormat, but your plugin seems to do everything I care about, except for delaying its writing of fileStates.txt until the very end of the build, which forced me to also have the autograder run then, which generates a "deprecated feature" warning from Gradle.

I suppose I could try to capture your stdout like I do with the Java compiler.

@sherter
Copy link
Owner

sherter commented Apr 19, 2019

I see. Generating reports for VerifyGoogleJavaFormat tasks certainly makes sense. However, this feature would be orthogonal to the fileStates cache. The cache isn't task specific. It's shared between all verify/format tasks (see

@Override
void apply(Project project) {
this.project = project
createExtension()
createDefaultTasks()
SharedContext context = new SharedContext(project, extension)
TaskConfigurator configurator = new TaskConfigurator(context)
project.gradle.taskGraph.beforeTask { Task task ->
if (task.project == this.project && task instanceof FormatTask) {
task.accept(configurator)
}
}
}
) and therefore not representative as a results report for one specific task.

We would need a properly defined schema for the report. What information should go in there? XML? JSON? What's the default file name pattern? We should probably have a look at all the other plugins that somehow generate reports and do it in a similar fashion.

I like the idea, but can't say if and when I will have time to implement it. If you are interested in contributing, I would be happy to comment on ideas and review code.

@danwallach
Copy link
Author

All the other tools I'm looking at produce XML and HTML reports: one for the humans and one for the machines. Let's focus on the machine-readable for now. The seemingly obvious thing is to produce XML that follows the same basic schema as the CSV that you're generating now: a list of entries, one per file, with various attributes for each file.

I don't know Groovy well enough to know if it's easier or harder to do JSON vs XML. On the other side, parsing the report, it's all the same, at least using a tool like Jackson.

The question is where the right place is to generate the report. I'm willing to take a swing at it, but I'm not sure where it best fits, e.g., whether report generation should be an option on the verify task or whether it should be its own task. JaCoCo has a separate task; Checkstyle doesn't.

@sherter
Copy link
Owner

sherter commented Apr 20, 2019

In my conception a report belongs to a verification task and therefore I would say that VerifyGoogleJavaFormat should implement Reporting<>. I haven't really used the JaCoCo plugin though and don't know what the rational behind having a separate task is.

@danwallach
Copy link
Author

For them, "reporting" can generate a huge directory tree of HTML, one per source file, so I can see the justification for breaking that out into a separate action. For your plugin, that's not particularly necessary.

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

No branches or pull requests

2 participants