JBPM-8858 [Stunner] Lienzo - Unit Testing for the core migration to n… #77
Conversation
There was a problem hiding this comment.
Hi @treblereel, @romartin,
I did a quick look at the code and commented some lines (mostly FIXME and TODOs). But also questions about Java version upgrade, copy/pasted code from 3rd parties and suites.
Also it is not fully clear to me what is what :) How to build those tests? Which version of lienzo-core should I use?
Thank you @treblereel, looks good otherwise :)
| /** | ||
| * @author Dmitrii Tikhomirov | ||
| * Created by treblereel 11/5/19 | ||
| */ |
| /** | ||
| * @author Dmitrii Tikhomirov | ||
| * Created by treblereel 10/30/19 | ||
| */ |
| /** | ||
| * @author Dmitrii Tikhomirov | ||
| * Created by treblereel 11/8/19 | ||
| */ |
| /** | ||
| * @author Dmitrii Tikhomirov | ||
| * Created by treblereel 10/30/19 | ||
| */ |
| /** | ||
| * @author Dmitrii Tikhomirov | ||
| * Created by treblereel 11/8/19 | ||
| */ |
| public final Point2DArray set(final int i, final Point2D p) { | ||
|
|
||
|
|
||
| /* if(i >= holder.size()) { |
There was a problem hiding this comment.
Commented code should be fixed or removed.
|
|
||
| private static final String toDataURL(final elemental2.dom.HTMLCanvasElement element) | ||
| { | ||
| return element.toDataURL(null); // @FIXME Make sure this accepts null (mdp) |
| return element.toDataURL(null); // @FIXME Make sure this accepts null (mdp) | ||
| } | ||
|
|
||
| // TODO other arguments, e.g. for image/jpeg The second argument, if it is a number in the range 0.0 to 1.0 inclusive, must be treated as the desired quality level. If it is not a number or is outside that range, the user agent must use its default value, as if the argument had been omitted. |
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ |
There was a problem hiding this comment.
Is it copy-pasted code from Google? Should we use it? CC @mdproctor @romartin
| LienzoCoreAttributesTest.class, | ||
| PointsTest.class, | ||
| PointsMockTest.class, | ||
| }) |
There was a problem hiding this comment.
Same about this Suite, we had it in the past and it was pain, should we introduce it again?
CC @romartin
|
Merged into romartin@7890405 Anyway KEEPING IT opened because of @hasys comments - please @treblereel we should take care of those |
…ative APIs
JIRA: https://issues.jboss.org/browse/JBPM-8858
depends on romartin/lienzo-core#1
@romartin