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

Untestable API #3088

Closed
l3r8yJ opened this issue Apr 13, 2024 · 8 comments
Closed

Untestable API #3088

l3r8yJ opened this issue Apr 13, 2024 · 8 comments
Assignees

Comments

@l3r8yJ
Copy link
Contributor

l3r8yJ commented Apr 13, 2024

TranspileMojo have this private method #cleanUpClasses, which is extremely hard to test, because it's private, obviously

private void cleanUpClasses(final Collection<? extends Path> java) {
final Set<Path> unexpected = java.stream()
.map(path -> this.generatedDir.toPath().relativize(path))
.map(TranspileMojo::classExtension)
.map(binary -> this.outputDir.toPath().resolve(binary))
.collect(Collectors.toSet());
for (final Path binary : unexpected) {
try {
synchronized (TranspileMojo.class) {
Files.deleteIfExists(binary);
}
} catch (final IOException cause) {
throw new IllegalStateException(
String.format("Can't delete file %s", binary),
cause
);
}
}
}

my suggestion is to create a separate class instead of private method, which will resolve same problem. Class can be moved to another repo or it can be placed inside of EO

@maxonfjvipon @volodya-lombrozo wdyt?

@maxonfjvipon
Copy link
Member

@l3r8yJ sounds good for me

@l3r8yJ
Copy link
Contributor Author

l3r8yJ commented Apr 14, 2024

@maxonfjvipon assign me please

@volodya-lombrozo
Copy link
Member

@maxonfjvipon @l3r8yJ, I have some doubts about this method. Actually, this is a crutch that we added to hide a problem with "unexpected" classes that were added during compilation:

Some libraries by mistake can put ALL their compiled classes right into the final library jar, instead of only adding atoms.

Therefore, I believe that we should solve the original problem instead of adding tests that we will remove as soon as we solve the problem.

@maxonfjvipon
Copy link
Member

@volodya-lombrozo I think we'll face this problem again while implementing #1602. Imagine you have two versioned objects from different libraries QQ.txt.sprintf|0.1.0 and QQ.files.dir|0.2.0 and they both needs their own eo-runtime. It may be the same version of runtime, may be different. And I'm not sure how it should be resovled.
For now we have two tickets: bad API and no concurrency test. Let's implement it for now.

@volodya-lombrozo
Copy link
Member

volodya-lombrozo commented Apr 14, 2024

@maxonfjvipon, this is exactly what I mentioned above: we don't know how to solve the original problem:

And I'm not sure how it should be resovled.

Let's then concentrate on this problem instead of implementing the code that we will remove immediately when we actually do solve the problem.

As for this:

For now we have two tickets: bad API and no concurrency test.

I have no idea what exactly you mean; could you provide some links, please? Especially to the bad API ticket. Which API are you referring to?

Again, I believe that we should solve the original problem instead. But of course, if you enjoy beating around the bush, I can't stop you here :)

@l3r8yJ
Copy link
Contributor Author

l3r8yJ commented Apr 14, 2024

@maxonfjvipon to be honest, I completely agree with @volodya-lombrozo, in such cases there is no point in doing such work. I think we need to work smarter, not harder; and focus on the root cause of the problem

wdyt?

@maxonfjvipon
Copy link
Member

@l3r8yJ as you wish. Then I suggest at least to replace "wrong" tickets and puzzles with "right" ones which would explain what we actually need to do and when it'll be possible

@l3r8yJ
Copy link
Contributor Author

l3r8yJ commented Apr 14, 2024

@maxonfjvipon okay

@l3r8yJ l3r8yJ closed this as not planned Won't fix, can't repro, duplicate, stale Apr 14, 2024
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

3 participants