Merge branch 'main' into pedro-dev

conjure-cp · Feb 19, 2024 · d27dfe3 · d27dfe3
2 parents 27bb092 + c89cdab
commit d27dfe3
Show file tree

Hide file tree

Showing 25 changed files with 336 additions and 219 deletions.
diff --git a/.github/workflows/code-coverage-deploy.yml b/.github/workflows/code-coverage-deploy.yml
@@ -97,6 +97,7 @@ jobs:
           commit-message: "Actions: Update coverage indexes"
 
   comment:
+    permissions: write-all
     needs: deploy-coverage
     runs-on: ubuntu-latest
     steps:
@@ -114,51 +115,6 @@ jobs:
           key: ${{ runner.os }}-${{ env.rust_release }}-${{ github.event.repository.updated_at }}
           restore-keys: ${{ runner.os }}-${{ env.rust_release }}
 
-      - name: Install rust ${{ env.rust_release }}
-        run: rustup update ${{ env.rust_release }} && rustup default ${{ env.rust_release }}
-
-      # from https://github.com/bewee/rustdoc-coverage-action-example/
-      - name: Fetch base
-        run: git fetch origin ${{ github.event.workflow_run.event.pull_request.base.sha }}
-
-      - name: Checkout base
-        run: git checkout ${{ github.event.workflow_run.event.pull_request.base.sha }}
-
-      - name: Calculate base doc coverage
-        uses: bewee/rustdoc-coverage-action@v1
-      - name: Fetch head
-        run: git fetch origin ${{ github.event.workflow_run.event.pull_request.head.sha }}
-      - name: Checkout head
-        run: git checkout ${{ github.event.workflow_run.event.pull_request.head.sha }}
-
-      - name: Calculate doc coverage
-        id: coverage
-        uses: bewee/rustdoc-coverage-action@v1
-
-      - name: Download code coverage info for main
-        run: | 
-          wget https://conjure-cp.github.io/coverage/main/lcov.info
-
-        
-      - name: Find coverage comment
-        uses: peter-evans/find-comment@v1
-        id: fc
-        with:
-          issue-number: ${{ github.event.workflow_run.event.pull_request.number }}
-          comment-author: "github-actions[bot]"
-          body-includes: "## Documentation Coverage"
-
-      - name: Delete coverage comment if it exists
-        if: steps.fc.outputs.comment-id != ''
-        uses: actions/github-script@v6
-        with:
-          script: |
-            github.rest.issues.deleteComment({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              comment_id: ${{ steps.fc.outputs.comment-id }}
-            })
-
       - name: Set shas
         id: sha
         uses: actions/github-script@v6
@@ -177,54 +133,103 @@ jobs:
             // whatever GITHUB_SHA is. 
             return callee_run.head_sha;
 
-      - name: Install lcov
-        run: |
-          sudo apt-get install -y lcov
+      - name: Download artifact
+        uses: dawidd6/action-download-artifact@v2
+        with:
+          name: code-coverage-${{ steps.sha.outputs.result }}
+          workflow: code-coverage.yml
+          path: ./deploy
+
+      - name: Get PR number
+        id: prnum
+        run: | 
+          echo "num=$(cat deploy/prnumber)" > $GITHUB_OUTPUT
+
+      - name: Install rust ${{ env.rust_release }}
+        run: rustup update ${{ env.rust_release }} && rustup default ${{ env.rust_release }}
 
-      - name: Generate lcov summary for main
-        id: lcov-main
+      - name: Generate lcov summary for main and pr
+        continue-on-error: true
+        id: lcov
         run: |
-          wget https://conjure-cp.github.io/coverage/main/lcov.info
-          echo "summary=$(lcov --summary lcov.info --exclude '.cargo/**/*.rs')" >> "$GITHUB_OUTPUT"
 
-      - name: Generate lcov summary for PR
-        id: lcov-pr
+          sudo apt-get install -y lcov
+          wget https://${{github.repository_owner}}.github.io/conjure-oxide/coverage/main/lcov.info
+          lcov --summary lcov.info > cov.txt
+          echo "main<<EOFABC" >> $GITHUB_OUTPUT
+          echo "$(cat cov.txt | tail -n +3)" >> $GITHUB_OUTPUT
+          echo 'EOFABC' >> $GITHUB_OUTPUT
+
+          lcov --summary ./deploy/lcov.info > cov.txt
+          echo "pr<<EOFABC" >> $GITHUB_OUTPUT
+          echo "$(cat cov.txt | tail -n +3)" >> $GITHUB_OUTPUT
+          echo 'EOFABC' >> $GITHUB_OUTPUT
+
+      - name: Get doc-coverage for main and pr
+        id: doccov 
         run: |
-          wget https://conjure-cp.github.io/coverage/${{steps.sha.outputs.result}}/lcov.info
-          echo "summary=$(lcov --summary lcov.info --exclude '.cargo/**/*.rs')" >> "$GITHUB_OUTPUT"
+          wget https://${{github.repository_owner}}.github.io/conjure-oxide/coverage/main/doc-coverage.txt
+          echo "main<<EOFABC" >> $GITHUB_OUTPUT
+          echo "$(cat doc-coverage.txt)" >> $GITHUB_OUTPUT
+          echo 'EOFABC' >> $GITHUB_OUTPUT
+          echo "pr<<EOFABC" >> $GITHUB_OUTPUT
+          echo "$(cat ./deploy/doc-coverage.txt)" >> $GITHUB_OUTPUT
+          echo 'EOFABC' >> $GITHUB_OUTPUT
 
+      - name: Find coverage comment
+        uses: peter-evans/find-comment@v1
+        continue-on-error: true
+        id: fc
+        with:
+          issue-number: ${{steps.prnum.outputs.num}}
+          comment-author: "github-actions[bot]"
+          body-includes: "## Documentation Coverage"
+
+      - name: Delete coverage comment if it exists
+        if: steps.fc.outputs.comment-id != ''
+        uses: actions/github-script@v6
+        with:
+          script: |
+            github.rest.issues.deleteComment({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              comment_id: ${{ steps.fc.outputs.comment-id }}
+            })
 
       - name: Create coverage comment
         uses: peter-evans/create-or-update-comment@v1
         with:
-          comment-id: ${{ steps.fc.outputs.comment-id }}
-          issue-number: ${{ github.event.workflow_run.event.pull_request.number }}
+          issue-number: ${{steps.prnum.outputs.num}}
           body: |
             ## Documentation Coverage
-            **${{steps.coverage.outputs.documented}}** documented (**${{steps.coverage.outputs.diff-documented}}**).
 
-            **${{steps.coverage.outputs.examples}}** with examples (**${{steps.coverage.outputs.diff-examples}}**).
+            <details>
+            <summary>This PR</summary>
+
+            ```
+            ${{steps.doccov.outputs.pr}}
+            ```
+
+            </details>
 
             <details>
-            <summary>Documentation Coverage Report</summary>
-            ${{steps.coverage.outputs.table}}
+            <summary>Main</summary>
+            
+            ```
+            ${{steps.doccov.outputs.main}}
+            ```
+
             </details>
 
             ## Code Coverage
 
-            **Summary**
+            [This PR](https://${{github.repository_owner}}.github.io/conjure-oxide/coverage/${{steps.sha.outputs.result}})
 
-            This PR:
             ```
-            ${{ steps.lcov-pr.outputs.summary }}
+            ${{steps.lcov.outputs.pr}}
             ```
 
-            Main:
-            ```
-            ${{ steps.lcov-main.outputs.summary }}
+            [Main](https://${{github.repository_owner}}.github.io/conjure-oxide/coverage/main)
             ```
-
-            **Full Coverage Reports**
-
-            * [This PR](https://conjure-cp.github.io/conjure-oxide/coverage/${{ steps.sha.outputs.result }}).
-            * [Main](https://conjure-cp.github.io/conjure-oxide/coverage/main).
+            ${{steps.lcov.outputs.main}}
+            ```
diff --git a/.github/workflows/code-coverage-main.yml b/.github/workflows/code-coverage-main.yml
@@ -44,22 +44,20 @@ jobs:
         run: |
           ./tools/coverage.sh
           
-      - name: Print documentation coverage to job summary
+      - name: Generate documentation coverage
         run: |
-           echo '```' >> $GITHUB_STEP_SUMMARY
-           RUSTDOCFLAGS='-Z unstable-options --show-coverage' cargo +nightly doc --workspace --no-deps | tee -a /dev/fd/2 >> $GITHUB_STEP_SUMMARY
-           echo '```' >> $GITHUB_STEP_SUMMARY
+          RUSTDOCFLAGS='-Z unstable-options --show-coverage' cargo +nightly doc --workspace --no-deps > doc-coverage.txt
 
-      - name: Generate documentation coverage JSON for diffs in PRs
+      - name: Generate documentation coverage JSON
         run: |
-           RUSTDOCFLAGS='-Z unstable-options --output-format json --show-coverage' cargo +nightly doc --workspace --no-deps > rustdoc-coverage-report.json
+          RUSTDOCFLAGS='-Z unstable-options --output-format json --show-coverage' cargo +nightly doc --workspace --no-deps > doc-coverage.json
 
-          
       - run: |
           mkdir -p deploy/
           cp -r target/debug/coverage/* deploy/     # html
           cp target/debug/lcov.info deploy/         # used for diffing code coverage in PR comments
-          cp rustdoc-coverage-report.json deploy/   # used for diffing doc coverage in PR comments
+          cp doc-coverage.json deploy/
+          cp doc-coverage.txt deploy/
           
       - name: Copy coverage report to /main.
         uses: JamesIves/github-pages-deploy-action@v4

diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml
@@ -58,10 +58,21 @@ jobs:
         run: |
           ./tools/coverage.sh
           
+      - name: Generate documentation coverage
+        run: |
+          RUSTDOCFLAGS='-Z unstable-options --show-coverage' cargo +nightly doc --workspace --no-deps > doc-coverage.txt
+
+      - name: Generate documentation coverage JSON
+        run: |
+          RUSTDOCFLAGS='-Z unstable-options --output-format json --show-coverage' cargo +nightly doc --workspace --no-deps > doc-coverage.json
+
       - run: |
           mkdir -p deploy/
           cp -r target/debug/coverage/* deploy/     # html
           cp target/debug/lcov.info deploy/         # used for comments
+          cp doc-coverage.json deploy/
+          cp doc-coverage.txt deploy/
+          echo "${{github.event.pull_request.number}}" > deploy/prnumber
 
       - name: Archive code coverage results for deployment
         uses: actions/upload-artifact@v3

diff --git a/.github/workflows/docs-generate.yml b/.github/workflows/docs-generate.yml
@@ -20,14 +20,8 @@ jobs:
       - name: Get Sha
         id: sha
         run: |
-          if [[ ${{ github.event_name }} == 'pull_request' ]] 
-          then
-            echo -e "sha=${{ github.event.pull_request.head.sha }}" >> "$GITHUB_OUTPUT"
-            echo -e "sha=${{ github.event.pull_request.head.sha }}"
-          else 
-            echo -e "sha=${{ github.sha }}" >> "$GITHUB_OUTPUT"
-            echo -e "sha=${{ github.sha }}"
-          fi
+          echo -e "sha=${{ github.sha }}" >> "$GITHUB_OUTPUT"
+          echo -e "sha=${{ github.sha }}"
 
       - uses: actions/cache@v3
         with:
@@ -59,5 +53,5 @@ jobs:
       - name: Archive code coverage results
         uses: actions/upload-artifact@v3
         with:
-          name: docs-${{ steps.sha.outputs.sha }}
+          name: docs-${{github.sha}}
           path: deploy/**
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/conjure_oxide/Cargo.toml b/conjure_oxide/Cargo.toml
@@ -15,7 +15,7 @@ conjure_rules = {path = "../crates/conjure_rules" }
 serde = { version = "1.0.196", features = ["derive"] }
 serde_json = "1.0.113"
 serde_with = "3.6.1"
-thiserror = "1.0.56"
+thiserror = "1.0.57"
 minion_rs = {path = "../solvers/minion" }
 anyhow = "1.0.79"
 clap = { version = "4.5.0", features = ["derive"] }

diff --git a/conjure_oxide/src/parse.rs b/conjure_oxide/src/parse.rs
@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 
+use conjure_core::metadata::Metadata;
 use serde_json::Value;
 
 use crate::ast::{Constant, DecisionVariable, Domain, Expression, Model, Name, Range};
@@ -246,14 +247,15 @@ fn parse_vec_op(
 
 fn parse_constant(constant: &serde_json::Map<String, Value>) -> Option<Expression> {
     match &constant["Constant"] {
-        Value::Object(int) if int.contains_key("ConstantInt") => {
-            Some(Expression::Constant(Constant::Int(
+        Value::Object(int) if int.contains_key("ConstantInt") => Some(Expression::Constant(
+            Metadata::new(),
+            Constant::Int(
                 int["ConstantInt"].as_array()?[1]
                     .as_i64()?
                     .try_into()
                     .unwrap(),
-            )))
-        }
+            ),
+        )),
         otherwise => panic!("Unhandled parse_constant {:#?}", otherwise),
     }
 }
diff --git a/conjure_oxide/src/rules/base.rs b/conjure_oxide/src/rules/base.rs
@@ -1,4 +1,6 @@
-use conjure_core::{ast::Constant as Const, ast::Expression as Expr, rule::RuleApplicationError};
+use conjure_core::{
+    ast::Constant as Const, ast::Expression as Expr, metadata::Metadata, rule::RuleApplicationError,
+};
 use conjure_rules::register_rule;
 
 /*****************************************************************************/
@@ -102,7 +104,7 @@ fn sum_constants(expr: &Expr) -> Result<Expr, RuleApplicationError> {
             let mut changed = false;
             for e in exprs {
                 match e {
-                    Expr::Constant(Const::Int(i)) => {
+                    Expr::Constant(metadata, Const::Int(i)) => {
                         sum += i;
                         changed = true;
                     }
@@ -112,7 +114,8 @@ fn sum_constants(expr: &Expr) -> Result<Expr, RuleApplicationError> {
             if !changed {
                 return Err(RuleApplicationError::RuleNotApplicable);
             }
-            new_exprs.push(Expr::Constant(Const::Int(sum)));
+            // TODO (kf77): Get existing metadata instead of creating a new one
+            new_exprs.push(Expr::Constant(Metadata::new(), Const::Int(sum)));
             Ok(Expr::Sum(new_exprs)) // Let other rules handle only one Expr being contained in the sum
         }
         _ => Err(RuleApplicationError::RuleNotApplicable),
@@ -302,10 +305,10 @@ fn remove_constants_from_or(expr: &Expr) -> Result<Expr, RuleApplicationError> {
             let mut changed = false;
             for e in exprs {
                 match e {
-                    Expr::Constant(Const::Bool(val)) => {
+                    Expr::Constant(metadata, Const::Bool(val)) => {
                         if *val {
                             // If we find a true, the whole expression is true
-                            return Ok(Expr::Constant(Const::Bool(true)));
+                            return Ok(Expr::Constant(metadata.clone(), Const::Bool(true)));
                         } else {
                             // If we find a false, we can ignore it
                             changed = true;
@@ -338,10 +341,10 @@ fn remove_constants_from_and(expr: &Expr) -> Result<Expr, RuleApplicationError>
             let mut changed = false;
             for e in exprs {
                 match e {
-                    Expr::Constant(Const::Bool(val)) => {
+                    Expr::Constant(metadata, Const::Bool(val)) => {
                         if !*val {
                             // If we find a false, the whole expression is false
-                            return Ok(Expr::Constant(Const::Bool(false)));
+                            return Ok(Expr::Constant(metadata.clone(), Const::Bool(false)));
                         } else {
                             // If we find a true, we can ignore it
                             changed = true;
@@ -370,7 +373,9 @@ fn remove_constants_from_and(expr: &Expr) -> Result<Expr, RuleApplicationError>
 fn evaluate_constant_not(expr: &Expr) -> Result<Expr, RuleApplicationError> {
     match expr {
         Expr::Not(contents) => match contents.as_ref() {
-            Expr::Constant(Const::Bool(val)) => Ok(Expr::Constant(Const::Bool(!val))),
+            Expr::Constant(metadata, Const::Bool(val)) => {
+                Ok(Expr::Constant(metadata.clone(), Const::Bool(!val)))
+            }
             _ => Err(RuleApplicationError::RuleNotApplicable),
         },
         _ => Err(RuleApplicationError::RuleNotApplicable),