application-services icon indicating copy to clipboard operation
application-services copied to clipboard

nimbus-fml: Make the output deterministic.

Open PieroV opened this issue 1 year ago • 1 comments
trafficstars

Changed some HashMaps with BTreeMaps to ensure the order of the output is deterministic.

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • [x] This PR follows the breaking change policy:
      • This PR has no breaking API changes, or
      • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
  • [x] Quality: This PR builds and tests run cleanly
    • Note:
      • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
      • If this pull request includes a breaking change, consider cutting a new release after merging.
  • [x] Tests: This PR includes thorough tests or an explanation of why it does not
    • It doesn't add new features, it changes a data type used here and there
    • Maybe it could be worth to include a test to check the output always follows a certain order, in case could you help me for that? :slightly_smiling_face:
  • [x] Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • [x] Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

Branch builds: add [firefox-android: branch-name] to the PR title.

PieroV avatar Jun 25 '24 15:06 PieroV

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 22.66%. Comparing base (8fd08c6) to head (db52b2b). Report is 522 commits behind head on main.

Files with missing lines Patch % Lines
...port/nimbus-fml/src/intermediate_representation.rs 0.00% 2 Missing :warning:
components/support/nimbus-fml/src/parser.rs 0.00% 2 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6283   +/-   ##
=======================================
  Coverage   22.66%   22.66%           
=======================================
  Files         332      332           
  Lines       29804    29804           
=======================================
  Hits         6754     6754           
  Misses      23050    23050           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov-commenter avatar Jun 25 '24 15:06 codecov-commenter

Thanks so much for doing this, @PieroV!

This looks great to me—though it looks like the test fixtures might need updating, from those Clippy and test failures 👀—but let's have Charlie and Beth give the official review.

linabutler avatar Jul 12 '24 18:07 linabutler

I tried to run cargo teston the rebased branch, but I can't find test failures, at most some tests are ignored.

Should I pass additional arguments?

PieroV avatar Jul 16 '24 13:07 PieroV