Nikodem Graczewski

I don't think this tests anything in the SUT.

I don't think this tests anything in the SUT.

Can we format those in a more readable way?

Can we format those in a more readable way?

OLMIS-6305: Recording for all failed feature will no be kept

OLMIS-6305: Updated jenkinsfile to archive recordings

OLMIS-6305: Restored the ability to run single feature locally

OLMIS-6305: Restored baseUrl

OLMIS-6305: Added recording for failed tests

OLMIS-6298: Refactored unit tests
OLMIS-6298: Refactored unit tests
OLMIS-6298: Refactored unit tests

OLMIS-6297: Refactored unit tests
OLMIS-6297: Refactored unit tests
Revert "OLMIS-6297: Refactored unit tests"

This reverts commit 1f6eac9283ce95e20b03d1c7453dfc635183108c.

    • -7
    • +14
    /src/openlmis-auth/auth-user.spec.js
    • -110
    • +88
    /src/openlmis-login/login.service.spec.js
  1. … 4 more files in changeset.
OLMIS-6297: Refactored unit tests

    • -14
    • +7
    /src/openlmis-auth/auth-user.spec.js
    • -88
    • +110
    /src/openlmis-login/login.service.spec.js
  1. … 4 more files in changeset.
OLMIS-6271: Refactored unit tests (part 2)
OLMIS-6271: Refactored unit tests (part 2)
OLMIS-6271: Refactored unit tests (part 2)

    • -94
    • +94
    /src/openlmis-modal/alert.service.spec.js
  1. … 45 more files in changeset.
According to the DRY principle, we should not be duplicating code, especially that much. 1. Instead of adding another data builder we could add a buildDto method which would abstract the exporting ...

According to the DRY principle, we should not be duplicating code, especially that much.
1. Instead of adding another data builder we could add a buildDto method which would abstract the exporting part. This way we have both fast and easily used builders as well as no code duplication.
2. So can be using the DataBuilder pattern and duplicating such a big chunk of the code. As long as the complexity is low, and it is the same for a separate builder and using the exporter pattern, we should be good.
3. Same is true using the exporter pattern.

Revert "OLMIS-6261: Refactored unit tests (part 1)"

This reverts commit a92390f79fb4dd1c1d8ca9e9823320ff03e014c7.

  1. … 32 more files in changeset.
Revert "OLMIS-6261: Commented out openlmisFileUpload directive tests"

This reverts commit 27bfe8833aa37cc406a8bddfe74af75d5971c3ad.

OLMIS-6261: Commented out openlmisFileUpload directive tests

OLMIS-6261: Refactored unit tests (part 1)
OLMIS-6261: Refactored unit tests (part 1)
This feels like a duplication of the RequisitionLineItemDataBuilder. Could we leverage the exporter pattern somehow? Same applies for similar DTO builders.

This feels like a duplication of the RequisitionLineItemDataBuilder. Could we leverage the exporter pattern somehow? Same applies for similar DTO builders.

OLMIS-6261: Refactored unit tests (part 1)

  1. … 32 more files in changeset.
Probably would be cool to propose it on some other channel. Dev forum perhaps?

Probably would be cool to propose it on some other channel. Dev forum perhaps?

Paweł Cieszko Why was the comment about adding DataBuilder interface removed? I think it is something worth discussing.

Paweł Cieszko Why was the comment about adding DataBuilder interface removed? I think it is something worth discussing.

As I said, it's our convention for creating objects without ID. The "build" methods return objects with ID. Since these are integration tests, I assume those objects are used for populating the dat...

As I said, it's our convention for creating objects without ID. The "build" methods return objects with ID. Since these are integration tests, I assume those objects are used for populating the database with the test data and thus can't have an ID.

Here's an example:
https://github.com/OpenLMIS/openlmis-referencedata/blob/master/src/test/java/org/openlmis/referencedata/testbuilder/GeographicZoneDataBuilder.java#L71