search

judovana / java-runtime-decompiler

GNU General Public License v3.0
68 stars 14 forks source link

Compile and overwrite test #175

Closed judovana closed 2 years ago

AurumTheEnd commented 2 years ago

I would suggest moving the "utility" classes to a different package in order to differentiate them from the actual org.jrd.backend.data test classes (CliTest, CompileUploadCliTest, but also ArchiveManagerOptionsTest). Thoughts?

judovana commented 2 years ago

not much agaist. What is the "AbstractAgentNeedingTest" then :)

-- Mgr. Jiri Vanek @.***

---------- Původní e-mail ---------- Od: Prokop Tunel @.> Komu: pmikova/java-runtime-decompiler @. github.com> Datum: 8. 10. 2021 10:23:18 Předmět: Re: [pmikova/java-runtime-decompiler] Compile and overwrite test (# 175) "

I would suggest moving the "utility" classes to a different package in order to differentiate them from the actual org.jrd.backend.data test classes ( CliTest, CompileUploadCliTest, but also ArchiveManagerOptionsTest). Thoughts?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub (https://github.com/pmikova/java-runtime-decompiler/pull/175#issuecomment-938445305) , or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAWFCS2PCEUSINNVNHB25M3UF2S67ANCNFSM5FRQV6GQ) . Triage notifications on the go with GitHub Mobile for iOS (https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or Android (https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub) . "

AurumTheEnd commented 2 years ago

Tests will fail until #111 is implemented. CompileUploadCliTest is currently essentially dependent on what plugins the user set up with the GUI, which is definitely unwanted testing behavior.

EDIT: it is too late for me to understand why the CI test passed. No idea. Dumbfounded. Perplexed. Astonished. Too many emotions.

EDIT EDIT: Oh yeah, in Model, we are creating PluginManager, which on fresh installs imports all the plugins because of ee08f0120636080312d14fba547526f570c9c34a. But because I don't have a fresh install, but am also missing some plugins because I don't use them, their tests fail locally. Very interesting and unintuitive behavior.

AurumTheEnd commented 2 years ago

The Windows test has been running for 10 minutes, that isn't good. Mainly because I will have to be the one fixing it.

judovana commented 2 years ago

Tests will fail until #111 is implemented. CompileUploadCliTest is currently essentially dependent on what plugins the user set up with the GUI, which is definitely unwanted testing behavior.

Right you are. I will add some assume. Btw, do you mind to check my usage of streams.get.... ? In the api test, the output of compilation is never here.

EDIT: it is too late for me to understand why the CI test passed. No idea. Dumbfounded. Perplexed. Astonished. Too many emotions.

yah:( I'm inclining to the bad unloading of agent.

EDIT EDIT: Oh yeah, in Model, we are creating PluginManager, which on fresh installs imports all the plugins because of ee08f01. But because I don't have a fresh install, but am also missing some plugins because I don't use them, their tests fail locally. Very interesting and unintuitive behavior.

Very interesting!

That was also my impression, that the plugin manager shoudl work with fresh install. And I have correct setup :)

AurumTheEnd commented 2 years ago

Btw, do you mind to check my usage of streams.get.... ?

I have not done a in-depth review yet. When I do in the next few days, I'll comment on it. Please don't merge this PR until then :D

EDIT: it is too late for me to understand why the CI test passed.

I'm inclining to the bad unloading of agent.

I was talking about the CompileAndUpload tests regarding different plugins. The passing overwrite test is a separate mystery.

AurumTheEnd commented 2 years ago

I see your 20 minute timeout still didn't help on Windows. Since all tests finish in under a minute on Ubuntu, this is a more serious issue than Windows just being slightly slower (which was the case prior to this PR - Windows build took about 1.5x the total time of the Ubuntu one).

judovana commented 2 years ago

I see your 20 minute timeout still didn't help on Windows. Since all tests finish in under a minute on Ubuntu, this is a more serious issue than Windows just being slightly slower (which was the case prior to this PR - Windows build took about 1.5x the total time of the Ubuntu one).

yy. remopved

judovana commented 2 years ago

I see your 20 minute timeout still didn't help on Windows. Since all tests finish in under a minute on Ubuntu, this is a more serious issue than Windows just being slightly slower (which was the case prior to this PR - Windows build took about 1.5x the total time of the Ubuntu one).

yy. remopved

see https://github.com/pmikova/java-runtime-decompiler/issues/182

Please fix in a separate PR.

AurumTheEnd commented 2 years ago

Please fix in a separate PR.

Nah. :)

judovana commented 2 years ago

yah!

-- Mgr. Jiri Vanek @.***

---------- Původní e-mail ---------- Od: Prokop Tunel @.> Komu: pmikova/java-runtime-decompiler @. github.com> Datum: 14. 10. 2021 22:51:56 Předmět: Re: [pmikova/java-runtime-decompiler] Compile and overwrite test (# 175) "

" Please fix in a separate PR. " Nah. :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub (https://github.com/pmikova/java-runtime-decompiler/pull/175#issuecomment-943722294) , or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAWFCS6H2QCNJYEWHP7GY4TUG47GNANCNFSM5FRQV6GQ) . Triage notifications on the go with GitHub Mobile for iOS (https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or Android (https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub) . "